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

Additional categories in DiffNode #160

Merged
merged 1 commit into from
Mar 7, 2016
Merged

Additional categories in DiffNode #160

merged 1 commit into from
Mar 7, 2016

Conversation

scompo
Copy link
Contributor

@scompo scompo commented Mar 7, 2016

Created a fresh PR from #159.

@SQiShER
Copy link
Owner

SQiShER commented Mar 7, 2016

This looks great. I was just about to merge, but the commit message doesn't explain what happened in this commit. Could you rephrase it so that it doesn't say "squashed changes" and instead describes the actual changes you made? I could probably rewrite it myself, but I'm not sure how to do that without disassociating the changes from your Github user. And that would be a shame.

The commit message doesn't need to be fancy. I usually stick to this structure:

Short summary (max. 72 chars wide) followed by a blank line

Long description if necessary (max. 72 chars per line, so it looks good in the git log)
Long description can have multiple lines.

You can change your message by running git commit --amend and when you're done just force-push it with git push --force.

Added DiffNode.addCategories(Collection<String> categories)
so users can add their own categories while traversing nodes.
Made DiffNode.getCategories return an UnmodifiableSet to discourage
users from editing the returned Set.
Implemented unit tests for categories manipulation in DiffNodeTest.
Added CategoriesTestIT integration test for categories resolution.
@scompo
Copy link
Contributor Author

scompo commented Mar 7, 2016

Hope this is fine, and thanks for the pointers on editing commits on git, I usually I just don't edit history, but it's a nice feature, I'll look into that in detail for the future 👍

SQiShER added a commit that referenced this pull request Mar 7, 2016
@SQiShER SQiShER merged commit b3d5c2e into SQiShER:master Mar 7, 2016
@scompo scompo changed the title Categories with squashed changes Additional categories in DiffNode Mar 7, 2016
@SQiShER
Copy link
Owner

SQiShER commented Mar 7, 2016

This is great! Thanks for this!

@scompo
Copy link
Contributor Author

scompo commented Mar 7, 2016

No problem, glad I could help :)
Just a question, do you think I should delete the branch from the old PR or should I leave it there for documentation reference?

@SQiShER
Copy link
Owner

SQiShER commented Mar 7, 2016

Once it has been merged, it isn't needed anymore. Feel free to delete it. 😃

@scompo
Copy link
Contributor Author

scompo commented Mar 7, 2016

Done, thanks 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants