Skip to content

generator: refactor implementation, part 1 highlights.#677

Merged
Insti merged 5 commits intoexercism:masterfrom
Insti:PR_641_approved
Jun 13, 2017
Merged

generator: refactor implementation, part 1 highlights.#677
Insti merged 5 commits intoexercism:masterfrom
Insti:PR_641_approved

Conversation

@Insti
Copy link
Copy Markdown
Contributor

@Insti Insti commented Jun 13, 2017

In #641 - generator: refactor implementation, part 1, Hilary started some generator refactorings. Progress on this seems to have stopped.

In order to continue the refactoring I have extracted from the uncontroversial changes from the initial PR.

I have further built upon these changes to continue the refactoring. (To follow in further PRs.)

hilary added 5 commits May 31, 2017 14:32
leftover from the proc days
Two reasons. Main reason: Implementation shouldn't be thinking in terms of
files. Secondary reason, old name was misleading, as it regenerates the file
when it exists :(
def initialize
@paths = FixturePaths
@slug = 'notemplate'
@exercise = Exercise.new(slug: 'notemplate')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there room here to use 'no template' or if necessary 'no_template'?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Probably. It would need to be no-template. This will need to happen later though as I'm trying not to mess with the pre-existing commits.

Copy link
Copy Markdown
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.

This looks really good. Glad these are being continued on.

@Insti Insti merged commit 1a55a8c into exercism:master Jun 13, 2017
@Insti Insti removed the ready label Jul 1, 2017
@Insti Insti deleted the PR_641_approved branch August 21, 2018 12:41
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