Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add clock problem #10

Merged
merged 1 commit into from
Jul 5, 2014
Merged

Add clock problem #10

merged 1 commit into from
Jul 5, 2014

Conversation

jrdnull
Copy link
Contributor

@jrdnull jrdnull commented Jul 5, 2014

Adds the clock problem from the other tracks. Is there a common standard to be used for the tests/examples, if not may I suggest http://www.php-fig.org/psr/. I will be happy to move the existing problems over and adjust the makefile to allow for the files to be renamed to the more commonly used form Class.php ClassTest.php

Let me know if there are any problems with this PR and I'll address them.

@kytrinyx
Copy link
Member

kytrinyx commented Jul 5, 2014

This looks great, thank you! I'm so excited that the PHP track is seeing some movement. It's been at a standstill for a while.

I don't think that we've talked about a common standard, but this is always welcome. I think the PSR seems like a good choice.

Thank you! ❤️ ❤️ ❤️

kytrinyx added a commit that referenced this pull request Jul 5, 2014
@kytrinyx kytrinyx merged commit 25b8e92 into exercism:master Jul 5, 2014
@wilmoore
Copy link

wilmoore commented Jul 6, 2014

Is there a common standard to be used for the tests/examples?

I'm not sure if we can call it a standard or not; however, what has been used across languages is the form "name_test.ext" and "example.ext". I realize that with PHP the PSR is sort of like the Python PEP; except, it isn't really "blessed" by the language creator nor the entire community. There is a vocal minority that favors it, but this isn't really a standard. If it were Python/PEP which is a standard for the language, then I'd say absolutely. In this case, I think having consistency between the x* languages is more useful.

That being said, final say-so goes to @kytrinyx.

@jrdnull
Copy link
Contributor Author

jrdnull commented Jul 6, 2014

I don't really think PHP-FIG can be considered a "vocal minority", just take a look at the member projects listed here. With regards to being consistent with the other x* languages Swift, C#, ObjC, Java etc all use a different naming style.

Should we not be completing these exercises in an idiomatic way for whichever language is in use?

@kytrinyx
Copy link
Member

kytrinyx commented Jul 6, 2014

what has been used across languages is the form "name_test.ext" and "example.ext"

No, it started out that way, but I changed the backend so that each language could name their files per their idioms. Some languages use CamelCase others stick things in subdirectories, etc.

Should we not be completing these exercises in an idiomatic way for whichever language is in use?

Yes, very much so!

I don't really care about consistency between languages, I'm more interested in consistency internally within the language. If there are tools that help someone do linting etc, then that is very useful.

I would say that for the tests/example code use PHP-FIG, but don't try to enforce it in the exercises/nitpicking on the site.

@jrdnull
Copy link
Contributor Author

jrdnull commented Jul 6, 2014

I would say that for the tests/example code use PHP-FIG, but don't try to enforce it in the exercises/nitpicking on the site.

Sounds good, I started a feature branch as I mentioned in a tweet over at https://github.com/jrdnull/xphp/tree/psr

I'm unsure on the best way to run the tests from the temp directory though, also currently the first test will only run with the rest being skipped, its not really proving that the examples solve the problem.

I've included Composer and PHPUnit configs to make use of Composer fetching PHPUnit and using the autoloader provided. I'm not sure what the cli tool would provide the user, ideally I think they should be provided with the phpunit.xml and composer.json so they can easily run the tests with:

% curl -sS https://getcomposer.org/installer | php
% php composer.phar install
% php vendor/bin/phpunit

If you don't want Composer included then we could provide our own autoloader or go back to requiring the files.

@kytrinyx
Copy link
Member

kytrinyx commented Jul 6, 2014

I'm not sure what the right approach is to run the skipped test. How about doing dirty file-munging and rewriting the file without the skip to run the tests?

@jrdnull
Copy link
Contributor Author

jrdnull commented Jul 6, 2014

I've got the tests running again, but I'm sure this can be done better than this

@wilmoore
Copy link

wilmoore commented Jul 7, 2014

No, it started out that way, but I changed the backend so that each language could name their files per their idioms. Some languages use CamelCase others stick things in subdirectories, etc.

Ah...I did not realize this had changed. Fair enough. 😃

may I suggest http://www.php-fig.org/psr

Which part? All of it, or like many project, just a subset (i.e. full PSR-2 or just PSR-1)?
Whatever we come up with, it would be good to get this on the README.

I started a feature branch as I mentioned in a tweet over at https://github.com/jrdnull/xphp/tree/psr

Cool. The one issue I have with this is that it is an epic change set. On one hand epic change sets are great; on the other hand, the review/change workflow is much more convoluted. Do we agree that breaking this out to multiple PRs is desirable?

@kytrinyx
Copy link
Member

kytrinyx commented Jul 7, 2014

Yeah, I would prefer to have PR's that let us think about things in chunks.

Usually I try to do one for adding pre-requisites and doing setup etc, then one change per exercise.

We can add a SETUP.md in the root of the repository with language-specific stuff that gets sewn into all the READMEs.

@wilmoore
Copy link

wilmoore commented Jul 7, 2014

👍

kytrinyx added a commit that referenced this pull request Aug 31, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants