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 helper scripts for creation of exercises #39

Merged
merged 47 commits into from
Oct 6, 2019

Conversation

pclausen
Copy link
Contributor

Adding create_fotran_test.py and CMakeLists.txt for easier creation of
new exercises.

Adding create_fotran_test.py and CMakeLists.txt for easier creation of
new exercises.
Copy link
Contributor

@SaschaMann SaschaMann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few general notes:

  • I think these files should be in bin/ and not config/ because they're scripts that you run when working on the track and not config files.
  • Running some kind of Python linter helps a lot to keep scripts consistent and to adhere to Python Best Practices. Most IDEs or editors have support or extensions to accomplish this. Makes the code a bit more readable for otherse overall.
  • A short comment what each function does would help future contributors figure out how the script works in case they need to debug something.

config/create_fortran_test.py Outdated Show resolved Hide resolved
config/create_fortran_test.py Show resolved Hide resolved
config/create_fortran_test.py Show resolved Hide resolved
config/create_fortran_test.py Show resolved Hide resolved
config/create_fortran_test.py Show resolved Hide resolved
# Prerequsites
# - Working cmake and fortran compiler
# - Python3.x (it may work with Python2 but I have not tested)
# - latest version of https://github.com/exercism/problem-specifications.git
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some tracks add the problem specs repo as a git submodule, so that it's easier to keep track of the version that was used to generate tests. A git submodule is essentially a reference to another git repo at a certain commit.

I think it's worth considering doing this to make it easier to keep the tests up-to-date if the problem specs change. I can go into further detail on Slack if you want.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some tracks require that the problem-specification repository be cloned at the same level as the language track.

So:

.                                                                                                                                               
├── automated-mentoring-support                                                                                                                 
├── bash                                                                                                                                        
├── fortran
├── haskell
├── problem-specifications
├── python
├── ruby
├── ruby-analyzer
├── vimscript
├── website
└── website-copy

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, here I am a bit out of my knowledge of git to do this. Seems overkill to me to create a new git sub-repo for a single file(?) But I leave this to you guys to decide.

Copy link
Contributor

@SaschaMann SaschaMann Aug 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A submodule isn't a new repo, it's a reference/link to a different repo at a certain commit. In the filesystem it's just a normal directory, but it allows you to refer to a specific version of that repo.

In this case the advantage of adding problem-specs as a submodule is that you could pull the latest version of it and update the exercises to that version and that everyone using the repo is on the same problem-specs version.
If problem-specs is updated after that, it's easy to find out what changed since the last time the exercises were updated, which can potentially make it easier to keep them up-to-date. The C# track does it, you can see what it looks like here: https://github.com/exercism/csharp/.

There are other ways to keep track of the changes to problem-specs, so it depends on your preferred workflow and is mostly a matter of preference.

I brought it up because I personally like that approach but perhaps it's best to ignore it for now and perhaps revisit it later once you have more experience with git and know what works better for you.

config/create_fortran_test.py Show resolved Hide resolved
pclausen and others added 6 commits August 15, 2019 22:11
Adding create_fotran_test.py and CMakeLists.txt for easier creation of
new exercises.
Added instructions about how to use the create_fotran_test.py script
for initial creation of a fortran _test.f90 file
Fixing lint-issues and added doc strings

According to "python3 -m pylint create_fortran_test.py":

Your code has been rated at 7.32/10 (previous run: 6.07/10, +1.25)
Co-Authored-By: Sascha Mann <git@mail.saschamann.eu>
Copy link
Member

@kotp kotp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's adopt the "One sentence per line" where we can for this track. It will make diffs clearer, while our editors can likely wrap to the desired line length locally for lines that are "too long".

It makes corrections easier, diffs less noisy, commits won't involve sentences that it doesn't need to, etc.

I have provided a small set of changes regarding the "One Sentence Per Line", but there are other things that need to be changed as well.

docs/INSTALLATION.md Outdated Show resolved Hide resolved
docs/INSTALLATION.md Outdated Show resolved Hide resolved
docs/INSTALLATION.md Outdated Show resolved Hide resolved
docs/INSTALLATION.md Outdated Show resolved Hide resolved
docs/INSTALLATION.md Outdated Show resolved Hide resolved
docs/INSTALLATION.md Outdated Show resolved Hide resolved
docs/INSTALLATION.md Outdated Show resolved Hide resolved
pclausen and others added 6 commits August 18, 2019 20:11
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Merge master and formatting changes to TESTS.md and INSTALLATION.md.
No real change of content.
Copy link
Contributor

@SaschaMann SaschaMann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. This should probably be merged using "Squash and Merge" to create a single commit instead of having 13 individual ones.

(Best to wait for @kotp's approval, though)

@SaschaMann SaschaMann requested a review from kotp October 1, 2019 12:47
kotp
kotp previously requested changes Oct 2, 2019
bin/create_fortran_test.py Outdated Show resolved Hide resolved
bin/create_fortran_test.py Outdated Show resolved Hide resolved
bin/create_fortran_test.py Outdated Show resolved Hide resolved
bin/create_fortran_test.py Outdated Show resolved Hide resolved
docs/INSTALLATION.md Outdated Show resolved Hide resolved
docs/INSTALLATION.md Outdated Show resolved Hide resolved
docs/TESTS.md Show resolved Hide resolved
docs/TESTS.md Outdated Show resolved Hide resolved
docs/TESTS.md Outdated Show resolved Hide resolved
testlib/TesterMain.f90 Outdated Show resolved Hide resolved
pclausen and others added 11 commits October 6, 2019 18:39
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
pclausen and others added 2 commits October 6, 2019 18:43
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
@pclausen pclausen dismissed kotp’s stale review October 6, 2019 16:46

All changes integrated

pclausen and others added 21 commits October 6, 2019 18:57
Adding create_fotran_test.py and CMakeLists.txt for easier creation of
new exercises.
Fixing lint-issues and added doc strings

According to "python3 -m pylint create_fortran_test.py":

Your code has been rated at 7.32/10 (previous run: 6.07/10, +1.25)
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
Co-Authored-By: Victor Goff <keeperotphones@gmail.com>
@pclausen pclausen merged commit 7d10ce9 into exercism:master Oct 6, 2019
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.

None yet

3 participants