From 9d385274e31abfbd19a24279902f4961fcc8e4e7 Mon Sep 17 00:00:00 2001 From: Roman Leventov Date: Wed, 17 Apr 2019 15:27:14 +0200 Subject: [PATCH] Move dev-related files and instructions to dev/ directory; add committer's instructions (#7279) * Add committer_readme.md * Clean up * Update, add PR merge action checklist * Move dev-related docs and files except CONTRIBUTING.md to dev/ directory; More committer's intructions * Add some accents * Move TeamCity instruction images to teamcity-images/, edit CONTRIBUTING.md * Add links to tags --- CONTRIBUTING.md | 16 +- dev/committer-instructions.md | 176 ++++++++++++++++++ .../druid_intellij_formatting.xml | 0 .../eclipse.importorder | 0 .../eclipse_formatting.xml | 0 .../intellij-sdk-config.jpg | Bin INTELLIJ_SETUP.md => dev/intellij-setup.md | 3 +- .../inspections_change_apply.png | Bin .../structural_search_dialog.png | Bin .../structural_search_find.png | Bin .../structural_search_inspection.png | Bin .../structural_search_inspection_add.png | Bin ci/README_TeamCity.md => dev/teamcity.md | 10 +- distribution/src/assembly/source-assembly.xml | 4 +- 14 files changed, 195 insertions(+), 14 deletions(-) create mode 100644 dev/committer-instructions.md rename druid_intellij_formatting.xml => dev/druid_intellij_formatting.xml (100%) rename eclipse.importorder => dev/eclipse.importorder (100%) rename eclipse_formatting.xml => dev/eclipse_formatting.xml (100%) rename intellij-sdk-config.jpg => dev/intellij-sdk-config.jpg (100%) rename INTELLIJ_SETUP.md => dev/intellij-setup.md (99%) rename {ci => dev/teamcity-images}/inspections_change_apply.png (100%) rename {ci => dev/teamcity-images}/structural_search_dialog.png (100%) rename {ci => dev/teamcity-images}/structural_search_find.png (100%) rename {ci => dev/teamcity-images}/structural_search_inspection.png (100%) rename {ci => dev/teamcity-images}/structural_search_inspection_add.png (100%) rename ci/README_TeamCity.md => dev/teamcity.md (92%) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2fa1e2207bb..38d20528373 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -23,15 +23,17 @@ When submitting a pull request (PR), please use the following guidelines: - Make sure your code respects existing formatting conventions. In general, follow the same coding style as the code that you are modifying. -- For Intellij you can import our code style settings xml: [druid_intellij_formatting.xml](https://github.com/apache/incubator-druid/raw/master/druid_intellij_formatting.xml). -- For Eclipse you can import our code style settings xml: [eclipse_formatting.xml](https://github.com/apache/incubator-druid/raw/master/eclipse_formatting.xml). +- For Intellij you can import our code style settings xml: [`druid_intellij_formatting.xml`]( + https://github.com/apache/incubator-druid/raw/master/dev/druid_intellij_formatting.xml). +- For Eclipse you can import our code style settings xml: [`eclipse_formatting.xml`]( + https://github.com/apache/incubator-druid/raw/master/dev/eclipse_formatting.xml). - Do add/update documentation appropriately for the change you are making. - If you are introducing a new feature you may want to first write about your idea - for feedback to [dev@druid.apache.org](https://lists.apache.org/list.html?dev@druid.apache.org). - Non-trivial features should include unit tests covering the new functionality. + for feedback to [dev@druid.apache.org](https://lists.apache.org/list.html?dev@druid.apache.org). Or create an issue + using "Feature/Change" template. Non-trivial features should include unit tests covering the new functionality. Open + a "Proposal" issue for large changes. - Bugfixes should include a unit test or integration test reproducing the issue. - Do not use author tags/information in the code. -- Always include license header on each java file your create. See [this example](https://github.com/apache/incubator-druid/blob/master/core/src/main/java/org/apache/druid/metadata/PasswordProvider.java) - Try to keep pull requests short and submit separate ones for unrelated features, but feel free to combine simple bugfixes/tests into one pull request. - Keep the number of commits small and combine commits for related changes. @@ -39,6 +41,8 @@ When submitting a pull request (PR), please use the following guidelines: - Keep formatting changes in separate commits to make code reviews easier and distinguish them from actual code changes. +You can find more developers' resources in [`dev/`](dev) directory. + ## GitHub Workflow 1. Fork the apache/incubator-druid repository into your GitHub account @@ -136,7 +140,7 @@ When submitting a pull request (PR), please use the following guidelines: committer that merges your change will rebase and squash it into a single commit before committing it to master. -# FAQ +## FAQ ### Help! I merged changes from upstream and cannot figure out how to resolve conflicts when rebasing! diff --git a/dev/committer-instructions.md b/dev/committer-instructions.md new file mode 100644 index 00000000000..d23e23f7cbe --- /dev/null +++ b/dev/committer-instructions.md @@ -0,0 +1,176 @@ + + +This document contains various instructions relevant for Druid committers. + +## PR and issue action item checklist for committers + +This checklist describes steps that every committer should take for their own issues and PRs and when they are the first +committer who visits an issue or a PR authored by a non-committer. + +1. Add appropriate tags to the PR, in particular: + + - [**`Design Review`**](https://github.com/apache/incubator-druid/labels/Design%20Review) - for changes that will be + hard to undo after they appear in some Druid release, and/or changes that will have lasting consequences in the + codebase. Examples: + - Major architectural changes or API changes + - Adding new or changing behaviour of existing query types (e. g. changing an algorithm behind some query type or + changing from floating point to double precision) + - Adding new or changing existing HTTP requests and responses (e. g. a new HTTP endpoint) + - Adding new or changing existing interfaces for extensions (`@PublicApi` and `@ExtensionPoint`) + - Adding new or changing existing server configuration parameter (e. g. altering the behavior of a config + parameter) + - Adding new or changing existing emitted metrics + - Other major changes + + PRs that flesh out community standards, checklists, policies, and PRs that change issue and PR templates, in other + words, many of the PRs tagged `Area - Dev` should often be tagged `Design Review` as well. + + The PR description should succinctly, but completely list all public API elements (`@PublicApi` or + `@ExtensionPoint`), configuration options, emitted metric names, HTTP endpoint paths and parameters that are added + or changed in the PR. If they are not listed, ask the PR author to update the PR description. + + - [**`Incompatible`**](https://github.com/apache/incubator-druid/labels/Incompatible) - for changes that alter public + API elements (`@PublicApi` or `@ExtensionPoint`), runtime configuration options, emitted metric names, HTTP endpoint + behavior, or server behavior in some way that affects one of the following: + + - Ability to do a rolling update [as documented](http://druid.io/docs/latest/operations/rolling-updates.html) + without needing any modifications to server configurations or query workload. + - Ability to roll back a Druid cluster to a prior version. + - Ability to continue using old Druid extensions without recompiling them. + + Note that no matter what, we must support the ability to do a rolling update somehow (even if some special care is + needed), and the ability to roll back to at least the immediate prior Druid version. If a change makes either one + of these impossible then it should be re-designed. + + All `Incompatible` PRs should be tagged `Design Review` too, but not vice versa: some `Design Review` issues, + proposals and PRs may not be `Incompatible`. + + - [**`Release Notes`**](https://github.com/apache/incubator-druid/labels/Release%20Notes) - for important changes + that should be reflected in the next Druid’s version release notes. Critically, those are changes that require some + server or query configuration changes made by Druid cluster operators to preserve the former cluster behaviour, i. e. + the majority of PRs tagged `Incompatible`. However, some `Incompatible` PRs may not need to be tagged + `Release Notes`, e. g. PRs that only change some extension APIs, because when building extensions with the newer + Druid version the incompatibility will be discovered anyway. + + Secondarily, PRs that add new features, improve performance or improve Druid cluster operation experience could + also be tagged `Release Notes` at your discretion. + + - [**`Bug`**](https://github.com/apache/incubator-druid/labels/Bug) / [**`Security`**]( + https://github.com/apache/incubator-druid/labels/Security) / [**`Feature`**]( + https://github.com/apache/incubator-druid/labels/Feature) / [**`Performance`**]( + https://github.com/apache/incubator-druid/labels/Performance) / [**`Refactoring`**]( + https://github.com/apache/incubator-druid/labels/Refactoring) / [**`Improvement`**]( + https://github.com/apache/incubator-druid/labels/Improvement) - can be used to distinguish between types of changes. + [**`Compatibility`**](https://github.com/apache/incubator-druid/labels/Compatibility) tag also falls into this + category, it's specifically for PRs that restore or improve compatibility with previous Druid versions if it was + inadvertently broken, or for changes that ensure forward compatibility with future Druid versions, forseening specific + changes that would otherwise break the compatibility. + + - [**`Development Blocker`**](https://github.com/apache/incubator-druid/labels/Development%20Blocker) - for changes + that need to be merged before some other PRs could even be published. `Development Blocker` PRs should be prioritized + by reviewers, so that they could be merged as soon as possible, thus not blocking somebody's work. + +2. Consider adding one or several **`Area -`** tags to the PR or issue. Consider [creating a new `Area -` tag]( +#creating-a-new-tag-on-github) if none of the existing `Area` tags is applicable to the PR or issue. + + - [`Area - Automation/Static Analysis`]( + https://github.com/apache/incubator-druid/labels/Area%20-%20Automation%2FStatic%20Analysis) - for any PRs and issues + about Checkstyle, forbidden-apis, IntelliJ inspections, code style, etc. Should also be used for PRs and issue + related to TeamCity CI problems. + - [`Area - Cache`](https://github.com/apache/incubator-druid/labels/Area%20-%20Cache) - for PRs and issues related to + Druid's query results cache (local or remote). Don't use for PRs that anyhow relate to caching in different contexts. + - [`Area - Dev`](https://github.com/apache/incubator-druid/labels/Area%20-%20Dev) - for PRs and issues related to the + project itself, such as adding developer's docs and checklists, Github issue and PR templates, Github-related issues. + Don't use for PRs and issues related to CI problems: use either `Area - Testing` for problems with Travis or + `Area - Automation/Static Analysis` for problems with TeamCity. PRs with `Area - Dev` tag should usually change files + in `dev/` or `.github/` directories. + - [`Area - Documentation`](https://github.com/apache/incubator-druid/labels/Area%20-%20Documentation) - for PRs and + issues about Druid's documentation for users and cluster operators. Don't use for PRs and issues about the + documentation of the Druid's development process itself: use `Area - Dev` for that purpose. Don't use for issues and + PR regarding adding internal design documentation and specification to code, usually, in the form of Javadocs or + comments (there is no specialized tag for this). + - [`Area - Lookups`](https://github.com/apache/incubator-druid/labels/Area%20-%20Lookups) - for PRs and issues + related to Druid's Query Time Lookups (QTL) feature. + - [`Area - Null Handling`](https://github.com/apache/incubator-druid/labels/Area%20-%20Null%20Handling) - for PRs and + issues related to the [Null Handling project](https://github.com/apache/incubator-druid/issues/4349). + - [`Area - Operations`](https://github.com/apache/incubator-druid/labels/Area%20-%20Operations) - for PRs and issues + related to Druid cluster operation process, for example, PRs adding more alerting, logging, changing configuration + options. + - [`Area - Query UI`](https://github.com/apache/incubator-druid/labels/Area%20-%20Query%20UI) - for issues that + mention or discuss the questions related to presenting Druid query results for human perception. + - [`Area - Querying`](https://github.com/apache/incubator-druid/labels/Area%20-%20Querying) - for any PRs and issues + related to the process of making data queries against Druid clusters, including the PRs and issues about query + processing and aggregators. + - [`Area - Segment Balancing/Coordination`]( + https://github.com/apache/incubator-druid/labels/Area%20-%20Segment%20Balancing%2FCoordination) - for PRs and issue + related to the process of loading and dropping segments in Druid clusters according to specified *rules*, and + balancing segments between Historical nodes in clusters. Coordinator node is responsible for both processes. This tag + is not called "Area - Coordinator" because Coordinator has some other duties that are not covered by this tag, for + example, compacting segments. + - [`Area - Testing`](https://github.com/apache/incubator-druid/labels/Area%20-%20Testing) - use for any PRs and + issues related to testing (including integration testing), Travis CI issues, and flaky tests. For flaky tests, also + add [`Flaky test`](https://github.com/apache/incubator-druid/labels/Flaky%20test) tag. + - [`Area - Zookeeper/Curator`](https://github.com/apache/incubator-druid/labels/Area%20-%20Zookeeper%2FCurator) - for + any PRs and issues related to ZooKeeper, Curator, and node discovery in Druid. + + +3. **Consider adding any `Bug` and `Security` PRs to the next Druid milestone** whenever they are important enough to +fix before the next release. This ensures that they will be considered by the next release manager as potential release +blockers. Please don't add PRs that are neither `Bug` nor `Security`-related to milestones until after they are +committed, to avoid cluttering the release manager's workflow. + +4. If the PR has obvious problems, such as an empty description or the PR fails the CI, ask the PR author to fix these +problems even if you don't plan to review the PR. + +5. If you create an issue that is relatively small and self-contained and you don't plan to work on it in the near +future, consider tagging it [**`Contributions Welcome`**]( +https://github.com/apache/incubator-druid/labels/Contributions%20Welcome) so that other people know that the issue is +free to pick up and is relatively easily doable even for those who are not very familiar with the codebase. + +## PR merge action item checklist + +1. Add the PR to the next release milestone. + +2. If the PR is labeled `Incompatible` and the next release milestone differs from the previous only in the "patch" +number (such as 0.10.1, while the previous release is 0.10 or 0.10.0), rename the next milestone so that it bumps the +"minor" number (e. g. 0.10 -> 0.11). + +3. Check that the issue addessed in the PR is closed automatically by Github. If it's not, close the issue manually. + +4. Consider thanking the author for contribution, especially a new contributor. + +## Creating a new tag on Github + +After creating a tag for your PR or issue, don't forget to take the following steps: + +1. Search existing PRs and issues and try to add the new tag to at least three of them. + +2. Describe the new tag in the [PR and issue action item checklist for committers]( +#pr-and-issue-action-item-checklist-for-committers) above. + +3. Although it is *not* necessary to preliminarily discuss creation of a new tag in the developers' mailing list, please +announce the new tag after you've created it at `dev@druid.apache.org`. + +## Become an Administrator of the Druid project in TeamCity + +Druid committers shall obtain a status of a [Druid project]( +https://teamcity.jetbrains.com/project.html?projectId=OpenSourceProjects_Druid) +administrator. See the [corresponding section in the document about TeamCity]( +teamcity.md#becoming-a-project-administrator) for details. \ No newline at end of file diff --git a/druid_intellij_formatting.xml b/dev/druid_intellij_formatting.xml similarity index 100% rename from druid_intellij_formatting.xml rename to dev/druid_intellij_formatting.xml diff --git a/eclipse.importorder b/dev/eclipse.importorder similarity index 100% rename from eclipse.importorder rename to dev/eclipse.importorder diff --git a/eclipse_formatting.xml b/dev/eclipse_formatting.xml similarity index 100% rename from eclipse_formatting.xml rename to dev/eclipse_formatting.xml diff --git a/intellij-sdk-config.jpg b/dev/intellij-sdk-config.jpg similarity index 100% rename from intellij-sdk-config.jpg rename to dev/intellij-sdk-config.jpg diff --git a/INTELLIJ_SETUP.md b/dev/intellij-setup.md similarity index 99% rename from INTELLIJ_SETUP.md rename to dev/intellij-setup.md index e3274114870..448160b9349 100644 --- a/INTELLIJ_SETUP.md +++ b/dev/intellij-setup.md @@ -18,7 +18,8 @@ --> # IntelliJ Setup -This document contains some examples and instructions on how to get IntelliJ setup to run local debugging and test setups of Druid. +This document contains some examples and instructions on how to get IntelliJ setup to run local debugging and test +setups of Druid. ## Project SDK diff --git a/ci/inspections_change_apply.png b/dev/teamcity-images/inspections_change_apply.png similarity index 100% rename from ci/inspections_change_apply.png rename to dev/teamcity-images/inspections_change_apply.png diff --git a/ci/structural_search_dialog.png b/dev/teamcity-images/structural_search_dialog.png similarity index 100% rename from ci/structural_search_dialog.png rename to dev/teamcity-images/structural_search_dialog.png diff --git a/ci/structural_search_find.png b/dev/teamcity-images/structural_search_find.png similarity index 100% rename from ci/structural_search_find.png rename to dev/teamcity-images/structural_search_find.png diff --git a/ci/structural_search_inspection.png b/dev/teamcity-images/structural_search_inspection.png similarity index 100% rename from ci/structural_search_inspection.png rename to dev/teamcity-images/structural_search_inspection.png diff --git a/ci/structural_search_inspection_add.png b/dev/teamcity-images/structural_search_inspection_add.png similarity index 100% rename from ci/structural_search_inspection_add.png rename to dev/teamcity-images/structural_search_inspection_add.png diff --git a/ci/README_TeamCity.md b/dev/teamcity.md similarity index 92% rename from ci/README_TeamCity.md rename to dev/teamcity.md index a567234abec..1fb15b334a1 100644 --- a/ci/README_TeamCity.md +++ b/dev/teamcity.md @@ -57,10 +57,10 @@ you an e-mail with a link to the ticket. Note that: 1. Open a structural search dialog: `Edit` -> `Find` -> `Search Structurally...` 2. Enter a pattern that you want to make an inspection, for example: -![Structural Search dialog](structural_search_dialog.png) +![Structural Search dialog](teamcity-images/structural_search_dialog.png) 3. Press the `Find` button to test your pattern: -![Structural Search find results](structural_search_find.png) +![Structural Search find results](teamcity-images/structural_search_find.png) Note that even if currently the pattern finds nothing, it might still be a good idea to add it as an inspection to prevent bugs creeping in the codebase in the future. However, test that your pattern doesn't contain mistakes by @@ -68,10 +68,10 @@ deliberately adding code with a mistake that should be spotted by the pattern in that your Structural Search pattern finds that newly added dummy mistake. 4. Open `Preferences...` -> `Inspections`, navigate to `General` -> `Structural Search inspection`: -![Structural Search inspection](structural_search_inspection.png) +![Structural Search inspection](teamcity-images/structural_search_inspection.png) 5. Click a button with a plus sign (`+`) in the bottom of the Options window, `Add Search Template...`: -![Structural Search inspection add](structural_search_inspection_add.png) +![Structural Search inspection add](teamcity-images/structural_search_inspection_add.png) 6. Click `OK`. Then you will see a dialong window with title `Save Template` and a field to enter a "template name". Enter in this field something reasonably short that yet would serve as a good message for people who add code that @@ -80,7 +80,7 @@ is caught by this pattern, e. g. "Use Map.putIfAbsent() instead of containsKey() 7. Move focus anywhere, e. g. by choosing any other inspection. Upon doing this, the `Apply` botton should become active: -![Inspections change apply](inspections_change_apply.png) +![Inspections change apply](teamcity-images/inspections_change_apply.png) 8. Press the `Apply` button, then `OK` to exit the inspection preferences. diff --git a/distribution/src/assembly/source-assembly.xml b/distribution/src/assembly/source-assembly.xml index 4dde8b5dac3..2426b47bd4b 100644 --- a/distribution/src/assembly/source-assembly.xml +++ b/distribution/src/assembly/source-assembly.xml @@ -37,8 +37,8 @@ %regex[(?!((?!${project.build.directory}/)[^/]+/)*src/)(.*/)?\.idea(/.*)?] %regex[(?!((?!${project.build.directory}/)[^/]+/)*src/)(.*/)?[^/]*\.iml] - eclipse_formatting.xml - eclipse.importorder + dev/eclipse_formatting.xml + dev/eclipse.importorder %regex[(?!((?!${project.build.directory}/)[^/]+/)*src/)(.*/)?pom\.xml\.releaseBackup]