Skip to content

HDDS-4737. Add ModifierOrder to checkstyle rules #1839

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.

Sign up for GitHub

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

Jump to bottom
Merged
adoroszlai merged 2 commits into from
Jan 30, 2021
Merged

HDDS-4737. Add ModifierOrder to checkstyle rules #1839

adoroszlai merged 2 commits into from
Jan 30, 2021

Conversation

@lamberken
Copy link
Member

@lamberken lamberken commented Jan 24, 2021

What changes were proposed in this pull request?

Add ModifierOrder to checkstyle rules

  • public final static to public static final
  • static public to public static
  • etc

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-4737

How was this patch tested?

No need new tests.

@amaliujia
Copy link
Contributor

amaliujia commented Jan 24, 2021

+1 seems be fine to enforce modifier order from now. The existing violation thus fix in this PR is not too large.

@lamberken
Copy link
Member Author

lamberken commented Jan 25, 2021

+1 seems be fine to enforce modifier order from now. The existing violation thus fix in this PR is not too large.

Thanks @amaliujia

@ayushtkn
Copy link
Member

ayushtkn commented Jan 25, 2021

Well this looks good to have, but this is changing the rules at the project level, I think such changes if required should be notified on the dev list.

FYI. Hadoop too had this checkstyle rule, but was disabled as part of HADOOP-12713

@amaliujia
Copy link
Contributor

amaliujia commented Jan 25, 2021

Sounds reasonable to at least mention this in dev list as it will impact all developers (though the impact is supposed to be minor)

@lamberken
Copy link
Member Author

lamberken commented Jan 26, 2021 •
edited
Loading

Got. @amaliujia @ayushtkn

@lamberken
Copy link
Member Author

lamberken commented Jan 27, 2021

hi @amaliujia @ayushtkn @elek, please review this pr again if you are free, thanks 😄

@lamberken
Copy link
Member Author

lamberken commented Jan 30, 2021

hi @adoroszlai , please review this pr again if you are free, thanks. If the merge operation is late, it will cause more conflicts.

ayushtkn
ayushtkn approved these changes Jan 30, 2021
View reviewed changes
Copy link
Member

@ayushtkn ayushtkn left a comment

Choose a reason for hiding this comment

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

There is no objection on the MailingList
We are good to go then. +1

@lamberken
Copy link
Member Author

lamberken commented Jan 30, 2021

There is no objection on the MailingList
We are good to go then. +1

Thanks 👍

adoroszlai
adoroszlai requested changes Jan 30, 2021
View reviewed changes
Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

Thanks @lamber-ken for proposing this. The meat of the change looks good.

adoroszlai
adoroszlai approved these changes Jan 30, 2021
View reviewed changes
Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

Thanks @lamber-ken for updating the patch.

@adoroszlai adoroszlai merged commit c426595 into apache:master Jan 30, 2021
@adoroszlai
Copy link
Contributor

adoroszlai commented Jan 30, 2021

Thanks @lamber-ken for the improvement, @amaliujia and @ayushtkn for the review.

@lamberken
Copy link
Member Author

lamberken commented Jan 30, 2021

Thanks @lamber-ken for the improvement, @amaliujia and @ayushtkn for the review.

Welcome.

errose28 added a commit to errose28/ozone that referenced this pull request Feb 1, 2021
@errose28
Merge branch 'master' into HDDS-4123
38913c9 
* master: (176 commits)
  HDDS-4760. Intermittent failure in ozone-ha acceptance test (apache#1853)
  HDDS-4770. Upgrade Ratis Thirdparty to 0.6.0 (apache#1868)
  HDDS-4765. Update close-pending workflow for new repo (apache#1856)
  HDDS-4737. Add ModifierOrder to checkstyle rules (apache#1839)
  HDDS-4704. Add permission check in OMDBCheckpointServlet (apache#1801)
  HDDS-4757. Unnecessary WARNING to set OZONE_CONF_DIR (apache#1849)
  HDDS-4751. TestOzoneFileSystem#testTrash failed when enabledFileSystemPaths and omRatisDisabled (apache#1851)
  HDDS-4736. Intermittent failure in testExpiredCertificate (apache#1838)
  HDDS-4758. Adjust classpath of ozone version to include log4j (apache#1850)
  HDDS-4518. Add metrics around Trash Operations. (apache#1832)
  HDDS-4708. Optimization: update RetryCount less frequently (update once per ~100) (apache#1805)
  HDDS-4748. sonarqube issue fix - "static" members should be accessed statically (apache#1748)
  HDDS-2402. Adapt hadolint check to improved CI framework (apache#1778)
  HDDS-4698. Upgrade Java for Sonar check (apache#1800)
  HDDS-4739. Upgrade Ratis to 1.1.0-eb66796d-SNAPSHOT (apache#1842)
  HDDS-4735. Fix typo in hdds.proto (apache#1837)
  HDDS-4430. OM failover timeout is too short (apache#1807)
  HDDS-4477. Delete txnId in SCMMetadataStoreImpl may drop to 0 after SCM restart. (apache#1828)
  HDDS-4688. Update Hadoop version to 3.2.2 (apache#1795)
  HDDS-4725. Change metrics unit from nanosecond to millisecond (apache#1823)
  ...
@lamberken lamberken deleted the patch branch February 7, 2021 06:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adoroszlai adoroszlai adoroszlai approved these changes

@ayushtkn ayushtkn ayushtkn approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@lamberken @amaliujia @ayushtkn @adoroszlai