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

Generator improvements. #485

Closed
Insti opened this issue Nov 8, 2016 · 17 comments
Closed

Generator improvements. #485

Insti opened this issue Nov 8, 2016 · 17 comments
Assignees
Labels

Comments

@Insti
Copy link
Contributor

Insti commented Nov 8, 2016

@petertseng found some things that could be improved while he was building a generator:

From: #484 (comment)

you need to touch .version or the generator won't run (increment_version requires that such a file exists). And you need to have an example file too. I tried to generate my tests before working on my example, but increment_version_in_example requires that the example actually exist.

@Insti
Copy link
Contributor Author

Insti commented Nov 8, 2016

@bmulvihill and I are working on improving the generator process, so we'll take these into account.

@bmulvihill
Copy link
Contributor

bmulvihill commented Nov 8, 2016

Yeah I actually noticed this too, we could have a simple command to scaffold this stuff when creating a new generator.

ie
bin/generate new EXERCISE_NAME

This will set up all the necessary files so all you need to do is implement the actual generator methods.

@bmulvihill
Copy link
Contributor

We might even want a different executable to handle something like this to prevent confusion, something like bin/scaffold EXERCISE_NAME

@kotp
Copy link
Member

kotp commented Nov 8, 2016

Maybe, but look at adding the functionality to take an argument. This seems so closely tied to generate that it does not make sense to have a new executable.

@petertseng
Copy link
Member

Another idea that could be considered if y'all really do not want a new command:

Make the two cases mentioned above tolerant of the files not existing.

  • In the case of the .version file: Create it and write 1 into it.
  • In the case of the example file: Either do nothing, or simply print a warning "example file doesn't exist" (since this case should only happen when writing the generator for the first time)

That said, something to scaffold the adding of a new exercise with generator seems useful - the lib file, the example.tt file, possibly appending it to config.json if possible (!). Back in the day it used to be more, you would have to add a bin/generate-#{slug}, thanks for getting rid of that step!

@kotp
Copy link
Member

kotp commented Nov 8, 2016

Prefer to create .version on first use. That is a great thing... No example file warning is good, as well, a first exercise contributor would be lead to do the needful.

@kotp kotp added this to the Test Generator milestone Jan 4, 2017
@kotp
Copy link
Member

kotp commented Mar 17, 2017

Should now use the version number from canonical rather than the sha as is currently done in bin/generator/git_command.rb Should add a warn('Deprecated - use canonical_version instead.') maybe?

@hilary
Copy link
Contributor

hilary commented Apr 21, 2017

@kotp and I are working on a generator, and have noticed some pain points.

  1. The documentation says a generator requires workload, however the method is actually work_load.
  2. It isn't at all clear how to construct work_load

Edit: having finished the generator, I now realize that it could be either (or, for that matter, joe) so long as example.tt and $PROBLEM_cases.rb use the same name.

@Insti
Copy link
Contributor Author

Insti commented Apr 21, 2017

I think it should be workload it's one word.

@Insti
Copy link
Contributor Author

Insti commented Apr 21, 2017

The documentation says a generator requires workload, however the method is actually work_load.

What made you think this? Was it some documentation? Another test file?
I'm trying to work out what we need to do to can clear up any confusion.

It isn't at all clear how to construct work_load

Yeah the documentation needs to be improved.

Thanks for the feedback @hilary

Edit: I read through the documentation in the readme, and can now better understand why you were confused!

@hilary
Copy link
Contributor

hilary commented Apr 21, 2017

One of the causes of my confusion was the inconsistency between #test_name (in $PROBLEM_cases) and #name (in example.tt). I left a note in your PR as well, @Insti

@hilary
Copy link
Contributor

hilary commented Apr 26, 2017

Currently these ideas of @Insti are buried in a PR. I wanted to surface them here so they are easier to find...

Now some of the things from the grand design in my head:

Note: "problem" in all these examples is the variable exercise name.

Generator problem_cases.rb should live in exercises/problem/.meta/generator/
Templates should live in problem/.meta/generator/ be called test_file_template.erb (or similar). Once they are under .meta they don't need to contain the word example
There definitely should be a default test_file_template.erb - probably in /lib/generator
ExerciseCase should not be an OpenStruct and the canonical data should be kept separate from the methods of this class.
Remove requirement for a test case object to know its index.
canonical_data version should be added to the test file - probably immediately to the left of the abbreviated commit hash.
.version should be updated to be the canonical_data version (e.g. 2.1.0) plus an x-ruby version. (3) e.g.: 2.1.0.3
Some of this will require changes to the bin/generator code.

By creating the new modern generators under .meta/generator it will be easy to track which exercises have been upgraded as the lib/problem_cases.rb are deleted with the end goal being all generators living under .meta/generator and lib being mostly empty.

Everything needed to create a generator for an exercise will be in exercises/problem/.meta/generator/

@Insti
Copy link
Contributor Author

Insti commented Apr 29, 2017

Todo

In checklist form so we can track our progress.

@hilary
Copy link
Contributor

hilary commented May 3, 2017

Question: ".version should be updated to be the canonical_data version (e.g. 2.1.0) plus an x-ruby version. (3) e.g.: 2.1.0.3".

Why do we want the x-ruby version? I'm assuming that the canonical data version is following semantic versioning. Understanding the role of the xruby version should (hopefully) make it possible to include it within the semver model.

@Insti
Copy link
Contributor Author

Insti commented May 3, 2017

We can't change the x-common version just because we do something.
If we were to add ruby-specific tests (there's a couple I would like to add things like input mutation checks to) we need to have a way of tracking those too.

If you want to use some other combination of symbols that is more semver compliant that is fine, just let me know what you want them to be.

@hilary
Copy link
Contributor

hilary commented May 3, 2017

I guess I'm asking why we don't just use the canonical data version? At least for exercises without any additional ruby-specific tests.

We could use semver for both and combine, where both are present. I'm just playing here, but an exercise with only canonical data might have a .version of

c1.0.2

an exercise with both canonical data and xruby data might have a .version of

c1.0.2+xr0.1.0

See also discussion from the CLI perspective: exercism/cli#362

@stale
Copy link

stale bot commented Jul 30, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot closed this as completed Aug 8, 2017
@Insti Insti removed the in progress label Jan 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants