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

Fix for duplicated comments #22

Merged
merged 11 commits into from
Jul 3, 2014

Conversation

zygm0nt
Copy link
Collaborator

@zygm0nt zygm0nt commented Jul 2, 2014

No description provided.

@coveralls
Copy link

Coverage Status

Coverage increased (+5.98%) when pulling f468b6d on zygm0nt:GITHUB-19-dont-repeat-comments into af7ae5d on TouK:master.


private Set<String> getSet() {
if (commentsCrcSet == null) {
commentsCrcSet = Sets.newHashSet();
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is sequivalent com.google.common.collect metod. I'm afraid of jcommander.internal.Sets - it's internal and I don't know it.

@ingwarsw
Copy link
Contributor

ingwarsw commented Jul 2, 2014

Mayby you also could try making #13 with that change (changes in same code?) ??

We have way too much comments now and that would help us a lot..

- Shortened crc
- Fixed NPE when no comments found
@ingwarsw
Copy link
Contributor

ingwarsw commented Jul 2, 2014

Just created you PR zygm0nt#1

- Now numbers < 0 should look way better
- Uppercased all letters
- Crc is not needed we are comparing whole line
- Repaired tests
@zygm0nt
Copy link
Collaborator Author

zygm0nt commented Jul 3, 2014

@ingwarsw thanks, I've merged that.

Conflicts:
	src/main/java/pl/touk/sputnik/connector/stash/StashFacade.java
@zygm0nt
Copy link
Collaborator Author

zygm0nt commented Jul 3, 2014

Please review changes - would like to merge this.

@coveralls
Copy link

Coverage Status

Coverage increased (+4.91%) when pulling 7f801c1 on zygm0nt:GITHUB-19-dont-repeat-comments into 95fe509 on TouK:master.

@SpOOnman
Copy link
Collaborator

SpOOnman commented Jul 3, 2014

Good work! Now it needs refactoring 👍

SpOOnman added a commit that referenced this pull request Jul 3, 2014
@SpOOnman SpOOnman merged commit f22e60b into TouK:master Jul 3, 2014
rufuslevi pushed a commit to rufuslevi/sputnik that referenced this pull request Mar 12, 2024
rufuslevi pushed a commit to rufuslevi/sputnik that referenced this pull request Mar 12, 2024
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.

None yet

4 participants