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

two-bucket: fix incorrect test case #941

Merged
merged 2 commits into from
Oct 10, 2017
Merged

Conversation

cmccandless
Copy link
Contributor

Resolves #911

Copy link
Member

@petertseng petertseng left a comment

Choose a reason for hiding this comment

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

as we discussed.

Copy link
Contributor

@Insti Insti left a comment

Choose a reason for hiding this comment

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

Thanks, I agree.

Copy link
Member

@rpottsoh rpottsoh left a comment

Choose a reason for hiding this comment

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

"version": probably should be updated.

Though there is a bit of a discussion taking place at #938, you may not need to update "version":.... Likely just the third digit would need to change, 1.1.0 to 1.1.1. It would only mean that any test generators (manual or automatic) should be re-run.

What do you think @petertseng and @Insti?

@Insti
Copy link
Contributor

Insti commented Oct 10, 2017

1.2.0 ?

The result of a test has changed, this may mean solutions that once passed now fail.
1.1.1 would be fixing a typo in the test name or similar.

(plus or minus whatever #938 decides.)

@rpottsoh
Copy link
Member

1.2.0 ?

Works for me. @cmccandless please change "version": to equal 1.2.0.

@cmccandless
Copy link
Contributor Author

Version updated to 1.2.0

@rpottsoh
Copy link
Member

This should be squashed when it is merged.

@cmccandless
Copy link
Contributor Author

@rpottsoh Would it be easier for me to go ahead and squash, or is there a simple way to do that from GitHub? I don't have much experience merging pull requests from command line, and that's mostly what I use.

@petertseng
Copy link
Member

Would it be easier for me to go ahead and squash, or is there a simple way to do that from GitHub?

You may squash via the command line if you like. Or you may leave it, and the one who merges the PR will squash, since GitHub provides the way to do so. I haven't yet seen a way that GitHub provides the submitter of the PR to squash via web, so if you want to do it you will be doing it via the command line.

You are welcome do to so, or not, as you please.

@cmccandless
Copy link
Contributor Author

cmccandless commented Oct 10, 2017 via email

@Insti Insti merged commit 8aa11e8 into exercism:master Oct 10, 2017
@Insti
Copy link
Contributor

Insti commented Oct 10, 2017

Squashed and merged, thanks @cmccandless ❤️

@cmccandless cmccandless deleted the patch-1 branch October 23, 2017 20:29
petertseng added a commit to exercism/rust that referenced this pull request Nov 20, 2018
)

Note that two-bucket was added to the Rust track at
#375
At the time it declared 1.0.0 compliance, but was actually compliant
with 1.2.0 already!

Further updates to 1.3.0 and 1.4.0 require no action on the Rust track's
part.

1.0.1: Fix exercise name
exercism/problem-specifications#715

Unversioned(!): Change descriptions
exercism/problem-specifications#716
(Our descriptions are no better or worse, so might as well keep them)

1.1.0: Add cases where goal equals one bucket's capacity
exercism/problem-specifications#763
Rust already had these cases, so just update the descriptions

1.2.0: Fix a test case added in 1.1.0
exercism/problem-specifications#911
exercism/problem-specifications#941
Rust already had the correct values, so no action.

1.3.0: move inputs to `input` object
exercism/problem-specifications#1084

1.4.0: rename JSON keys to camelCase
exercism/problem-specifications#1136
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.

4 participants