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

Scale generator README fixes #928

Closed
wants to merge 2 commits into from
Closed

Conversation

r4f4
Copy link
Contributor

@r4f4 r4f4 commented Feb 27, 2020

SSIA

r4f4 added 2 commits February 27, 2020 21:41
The notes were being rendered in the same line, not in separate ones as
originally intended.

Signed-off-by: Rafael Fonseca <[email protected]>
Also add a definition in terms of steps to make it more clear.

Signed-off-by: Rafael Fonseca <[email protected]>
@coriolinus
Copy link
Member

Thanks for contributing this!

The CI failure is at least partially not your fault; see #929 for details. You may need to rebase this PR once that one is merged. Still, CI will need to pass before we can consider merging.

I note that in several instances you have added trailing whitespace at the end of lines. This may or may not be your fault. If the trailing whitespace derives from the problem specifications, CI will mandate that we include it. If it's something you added, you'll need to remove it.

I haven't been tracking the state of the upstream specifications repo very closely, so I'm not sure whether or not this PR is tracking changes made there. If it is not, then we won't be able to merge this until the changes are also made there: CI ensures that the README files stay synchronized with those specifications. If it is in fact tracking changes there, you'll also need to update the version field in Cargo.toml appropriately.

@r4f4
Copy link
Contributor Author

r4f4 commented Feb 28, 2020

I note that in several instances you have added trailing whitespace at the end of lines. This may or may not be your fault. If the trailing whitespace derives from the problem specifications, CI will mandate that we include it. If it's something you added, you'll need to remove it.

The trailing whitespaces are there to create a new line. If there is a better way to do that, let me know.

I haven't been tracking the state of the upstream specifications repo very closely, so I'm not sure whether or not this PR is tracking changes made there. If it is not, then we won't be able to merge this until the changes are also made there: CI ensures that the README files stay synchronized with those specifications. If it is in fact tracking changes there, you'll also need to update the version field in Cargo.toml appropriately.

Oh I see. I'll send a PR against the problem-specifications repo then.

@coriolinus
Copy link
Member

If there is a better way to do that, let me know.

The better way depends on your intent. In these cases, I'd probably go with unordered lists, personally.

@r4f4
Copy link
Contributor Author

r4f4 commented Feb 28, 2020

I see issue exercism/problem-specifications#1640 is already tracking the formatting and I created exercism/problem-specifications#1642 for the typo. I'm closing this PR.

@r4f4 r4f4 closed this Feb 28, 2020
@coriolinus
Copy link
Member

Oh, very good. Thanks for putting in this work!

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.

2 participants