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

Add @SuppressModernizer #86

Merged
merged 1 commit into from Feb 1, 2019
Merged

Add @SuppressModernizer #86

merged 1 commit into from Feb 1, 2019

Conversation

stevie400
Copy link
Contributor

This add @SuppressModernizer to fix #3. I can update the docs once this approach is approved.

@gaul
cc @jhaber @kmclarnon

@gaul
Copy link
Owner

gaul commented Jan 30, 2019

@stevegutz This seems like a better approach than #74; thank you for reworking this! Could you resolve the conflicts and remove some of the whitespace changes before we continue?

@stevie400
Copy link
Contributor Author

Fixed whitespace, addressed merge conflicts, updated readme, and sorted out the checkstyle dependency mess.

@gaul
Copy link
Owner

gaul commented Jan 31, 2019

GitHub still reports conflicts; please squash these and then run:

git push origin +suppress-annotation

@@ -0,0 +1,119 @@
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
Copy link
Owner

Choose a reason for hiding this comment

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

Did you create this via git mv? It doesn't seem to retain the history.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a known github issue: isaacs/github#900

@stevie400
Copy link
Contributor Author

I'm not seeing any conflicts: screenshot. Perhaps you need to hard refresh the page?

@gaul
Copy link
Owner

gaul commented Feb 1, 2019

I still see conflicts:

image

Could you also squash the commits?

@stevie400 stevie400 force-pushed the suppress-annotation branch 2 times, most recently from 64a78e4 to 52922d5 Compare February 1, 2019 18:45
@stevie400
Copy link
Contributor Author

Rebased and squashed.

It was showing the merge conflicts because you're looking to rebase and merge. There was seemingly no way for them to be visible to me. It might be helpful for you to add a PR template so that contributors are aware of your expectations w/r/t the commit history.

@gaul gaul merged commit be66312 into gaul:master Feb 1, 2019
@gaul
Copy link
Owner

gaul commented Feb 1, 2019

Thank you for your contribution @stevegutz! This was a much-requested feature and I appreciate the extra effort to rework the implementation. I pushed 1.8.0-SNAPSHOT and will hope to push 1.8.0 release in a few days.

@stevie400
Copy link
Contributor Author

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.

disable Modernizer via @SuppressWarnings annotation
2 participants