| //// |
| Licensed to the Apache Software Foundation (ASF) under one or more |
| contributor license agreements. See the NOTICE file distributed with |
| this work for additional information regarding copyright ownership. |
| The ASF licenses this file to You under the Apache License, Version 2.0 |
| (the "License"); you may not use this file except in compliance with |
| the License. You may obtain a copy of the License at |
| |
| http://www.apache.org/licenses/LICENSE-2.0 |
| |
| Unless required by applicable law or agreed to in writing, software |
| distributed under the License is distributed on an "AS IS" BASIS, |
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. |
| See the License for the specific language governing permissions and |
| limitations under the License. |
| //// |
| For Committers |
| ============== |
| |
| image::business-gremlin.png[width=400] |
| |
| The guidelines that follow apply to those with commit access to the main repository: |
| |
| Communication |
| ------------- |
| |
| TinkerPop has a link:http://groups.google.com/group/gremlin-users[user mailing list] and a |
| link:https://lists.apache.org/list.html?dev@tinkerpop.apache.org[developer mailing list]. As a committer, |
| it is a good idea to join both. |
| |
| It would also be helpful to join the public link:https://s.apache.org/tinkerpop[TinkerPop HipChat room] for developer |
| discussion. This helps contributors to communicate in a more real-time way. Anyone can join as a guest, but for |
| regular contributors it may be best to request that an Apache HipChat account be created. |
| |
| Occasionally, online meetings via video conference are held. These meetings are schedule via the dev mailing list |
| about a week before they are to occur to find a day and time that is available for those interested in attending. |
| On the day of the meeting, the meeting organizer will create a Google Hangout (or similar video conferencing link) to |
| post to the TinkerPop room in HipChat. At that point, all who are interested can attend. Meeting minutes should be |
| taken and added to the <<meetings,Meetings>> section of this document using the pattern already established. |
| |
| Release Notes |
| ------------- |
| |
| There is a two-pronged approach to maintaining the change log and preparing the release notes. |
| |
| 1. For work that is documented in JIRA, run the release notes report to include all of |
| the tickets targeted for a specific release. This report can be included in the |
| release announcement. |
| |
| 2. The manual change log (`CHANGELOG.asciidoc`) can be used to highlight large |
| changes, describe themes (e.g. "We focused on performance improvements") or to |
| give voice to undocumented changes. |
| |
| Given the dependence on the JIRA report for generating additions to the `CHANGELOG.asciidoc`, |
| which uses the title of the issue as the line presented in the release note report, titles should |
| be edited prior to release to be useful in that context. In other words, an issue title should |
| be understandable as a change in the fewest words possible while still conveying the gist of the |
| change. |
| |
| Changes that break the public APIs should be marked with a "breaking" label and should be |
| distinguished from other changes in the release notes. |
| |
| Branches |
| -------- |
| |
| The "master" branch is used for the main line of development and release branches are constructed as needed |
| for ongoing maintenance work. If new to the project or are returning to it after some time away, it may be good |
| to send an email to the developer mailing list (or ask on HipChat) to find out what the current operating branches |
| are. |
| |
| Other branches may be created for collaborating on features or for RFC's that other developers may want to inspect. |
| It is suggested that the JIRA issue ID be used as the prefix, since that triggers certain automation, and it provides a |
| way to account for the branch lifecycle, i.e. "Who's branch is this, and can I delete it?" |
| |
| For branches that are NOT associated with JIRA issues, developers should utilize their Apache ID as |
| a branch name prefix. This provides a unique namespace, and also a way to account for the branch lifecycle. |
| |
| Developers should remove their own branches when they are no longer needed. |
| |
| Tags |
| ---- |
| |
| Tags are used for milestones, release candidates, and approved releases. Please refrain from creating arbitrary |
| tags, as they produce permanent clutter. |
| |
| Issue Tracker Conventions |
| ------------------------- |
| |
| TinkerPop uses Apache JIRA as its link:https://issues.apache.org/jira/browse/TINKERPOP[issue tracker]. JIRA is a |
| very robust piece of software with many options and configurations. To simplify usage and ensure consistency across |
| issues, the following conventions should be adhered to: |
| |
| * An issue's "status" should generally be in one of two states: `open` or `closed` (`reopened` is equivalent to `open` |
| for our purposes). |
| ** An `open` issue is newly created, under consideration or otherwise in progress. |
| ** A `closed` issue is completed for purposes of release (i.e. code, testing, and documentation complete). |
| ** Issues in a `resolved` state should immediately be evaluated for movement to `closed` - issue become `resolved` |
| by those who don't have the permissions to `close`. |
| * An issue's "type" should be one of two options: `bug` or `improvement`. |
| ** A `bug` has a very specific meaning, referring to an error that prevents usage of TinkerPop AND does not have a |
| reasonable workaround. Given that definition, a `bug` should generally have very high priority for a fix. |
| ** Everything else is an `improvement` in the sense that any other work is an enhancement to the current codebase. |
| * The "component" should be representative of the primary area of code that it applies to and all issues should have |
| this property set. |
| * Issues are not assigned "labels" with two exceptions: |
| ** The "breaking" label which marks an issue as one that is representative of a change in the API that might |
| affect users or providers. This label is important when organizing release notes. |
| ** The "deprecation" label which is assigned to an issue that is about removing a deprecated portion of the API. |
| * The "affects/fix version(s)" fields should be appropriately set, where the "fix version" implies the version on |
| which that particular issue will completed. This is a field usually only set by committers. |
| * The "priority" field can be arbitrarily applied with one exception. The "trivial" option should be reserved for |
| tasks that are "easy" for a potential new contributor to jump into and do not have significant impact to urgently |
| required improvements. |
| |
| Code Style |
| ---------- |
| |
| Contributors should examine the current code base to determine what the code style patterns are and should match their |
| style to what is already present. Of specific note however, TinkerPop does not use "import wildcards" - IDEs should |
| be adjusted accordingly to not auto-wildcard the imports. |
| |
| Build Server |
| ------------ |
| |
| TinkerPop uses both link:https://travis-ci.com/[Travis] and link:https://www.appveyor.com/[AppVeyor] for |
| link:https://en.wikipedia.org/wiki/Continuous_integration[CI] services. Travis validates builds on Ubuntu, while |
| AppVeyor validates builds on Windows. The build statuses can be found here: |
| |
| * link:https://travis-ci.org/apache/tinkerpop[Travis Build Status] |
| * link:https://ci.appveyor.com/project/ApacheSoftwareFoundation/tinkerpop[AppVeyor Build Status] |
| |
| Note that the CI process does not run integration tests or include Neo4j-related tests as those tests would likely |
| exceed the allowable times for builds on these servers. |
| |
| Deprecation |
| ----------- |
| |
| When possible, committers should avoid direct "breaking" change (e.g. removing a method from a class) and favor |
| deprecation. Deprecation should come with sufficient documentation and notice especially when the change involves |
| public APIs that might be utilized by users or implemented by providers: |
| |
| * Mark the code with the `@Deprecated` annotation. |
| * Use javadoc to further document the change with the following content: |
| ** `@deprecated As of release x.y.z, replaced by {@link SomeOtherClass#someNewMethod()}` - if the method is not |
| replaced then the comment can simply read "not replaced". Additional comments that provide more context are |
| encouraged. |
| ** `@see <a href="https://issues.apache.org/jira/browse/TINKERPOP-XXX">TINKERPOP-XXX</a>` - supply a link to the |
| JIRA issue for reference. |
| * Be sure that deprecated methods are still under test - consider using javadoc/comments in the tests themselves to |
| call out this fact. |
| * Create a new JIRA issue to track removal of the deprecation for future evaluation - this issue should have the |
| "breaking" label as well as a "deprecation" label. |
| * Update the "upgrade documentation" to reflect the API change and how the reader should resolve it. |
| |
| The JIRA issues that track removal of deprecated methods should be periodically evaluated to determine if it is |
| prudent to schedule them into a release. |
| |
| Developing Tests |
| ---------------- |
| |
| TinkerPop has a wide variety of test types that help validate its internal code as well as external provider code. |
| There are "unit tests" and "integration tests". Unit tests execute on standard runs of `mvn clean install`. These |
| tests tend to run quickly and provide a reasonable level of coverage and confidence in the code base. Integration |
| tests are disabled by default and must be explicitly turned on with a special build property by adding |
| `-DskipIntegrationTests=false` to the `mvn` execution. Integration tests run slower and may require external |
| components to be running when they are executed. They are "marked" as separate from unit tests by inclusion of the |
| suffix "IntegrateTest". |
| |
| Here are some other points to consider when developing tests: |
| |
| * Avoid use of `println` in tests and prefer use of a SLF4j `Logger` instance so that outputs can be controlled in a |
| standard way. |
| * If it is necessary to create files on the filesystem, do not hardcode directories - instead, use the `TestHelper` to |
| create directory structures. `TestHelper` will properly create file system structure in the appropriate build |
| directory thus allowing proper clean-up between test runs. |
| * If writing tests in one of the test suites, like `gremlin-test`, it is important to remember that if a new `Graph` |
| instance is constructed within the test manually, that it be closed on exit of that test. Failing to do this cleanup |
| can cause problems for some graph providers. |
| * Tests that are designed to use a `GraphProvider` implementation in conjunction with `AbstractGremlinTest` _and_ are |
| in the `/test` directory should not be named with `Test` as the suffix, as this will cause them to execute in some |
| environments without a `GraphProvider` being initialized by a suite. These types of tests should be suffixed with |
| `Check` instead. Please see link:https://github.com/apache/tinkerpop/blob/e32a4187e4f25e290aabe14007f9087c48a06521/neo4j-gremlin/src/test/java/org/apache/tinkerpop/gremlin/neo4j/structure/NativeNeo4jStructureCheck.java[NativeNeo4jStructureCheck] |
| for an example. |
| |
| Gremlin Language Test Cases |
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
| |
| When writing a test case for a Gremlin step, be sure to use the following conventions. |
| |
| * The name of the traversal generator should start with `get`, use `X` for brackets, `_` for space, and the Gremlin-Groovy sugar syntax. |
| ** `get_g_V_hasLabelXpersonX_groupXaX_byXageX_byXsumX_name()` |
| * When creating a test for a step that has both a barrier and sideEffect form (e.g. `group()`, `groupCount()`, etc.), test both representations. |
| ** `get_g_V_groupCount_byXnameX()` |
| ** `get_g_V_groupCountXaX_byXnameX_capXaX()` |
| * The name of the actual test case should be the name of the traversal generator minus the `get_` prefix. |
| * The Gremlin-Groovy version of the test should use the sugar syntax in order to test sugar (as Gremlin-Java8 tests test standard syntax). |
| ** `g.V.age.sum` |
| * Avoid using lambdas in the test case unless that is explicitly what is being tested as OLAP systems will typically not be able to execute those tests. |
| * `AbstractGremlinProcessTest` has various static methods to make writing a test case easy. |
| ** `checkResults(Arrays.asList("marko","josh"), traversal)` |
| ** `checkMap(new HashMap<String,Long>() {{ put("marko",1l); }}, traversal.next())` |
| |
| Developing Benchmarks |
| --------------------- |
| |
| Benchmarks are a useful tool to track performance between TinkerPop versions and also as tools to aid development |
| decision making. TinkerPop uses link:http://openjdk.java.net/projects/code-tools/jmh/[OpenJDK JMH] for benchmark development. |
| The JMH framework provides tools for writing robust benchmarking code that avoid many of the pitfalls inherent in benchmarking |
| JIT compiled code on the JVM. Example JMH benchmarks can be found |
| link:http://hg.openjdk.java.net/code-tools/jmh/file/tip/jmh-samples/src/main/java/org/openjdk/jmh/samples/[here]. |
| |
| TinkerPop benchmarks live in the `gremlin-benchmark` module and can either be run from within your IDE or as a standalone |
| uber-jar. The uber-jar is the JMH recommended approach and also makes it easy to distribute artifacts to various environments |
| to gather benchmarking numbers. Having said that, in most cases it should be sufficient to run it from within the IDE. |
| |
| Benchmarks will not run by default because they are time consuming. To enable benchmarks during the test phase do |
| `-DskipBenchmarks=false`. To change the number of warmup iterations, measurement iterations, and forks you can do |
| `mvn clean test -DskipBenchmarks=false -DdefaultForks=5 -DmeasureIterations=20 -DwarmupIterations=20`. Benchmark results |
| will be output by default to the `benchmarks` directory in JSON format. |
| |
| Benchmarks may also be run from the command line using the JMH runner. Build the uber-jar and simply run |
| `java -jar gremlin-benchmark-TP-VERSION.jar`. To see a list of JMH runner options, add the `-h` flag. |
| |
| The JUnit/JMH integration was inspired by the Netty projects microbenchmarking suite. Please refer to the Netty |
| link:http://netty.io/wiki/microbenchmarks.html[docs] for more details. Presently there are 3 abstract benchmark classes |
| that may be used as building blocks for your benchmarks; `AbstractBenchmarkBase`, `AbstractGraphBenchmark`, and |
| `AbstractGraphMutateBenchmark`. |
| |
| * `AbstractBenchmarkBase` - extend when your benchmark does not require a graph instance |
| * `AbstractGraphBenchmark` - extend when you are benchmarking read operations against a graph |
| * `AbstractGraphMutateBenchmark` - extend when you are benchmarking graph mutation operations eg. `g.addV()`, `graph.addVertex()` |
| |
| [[rtc]] |
| Review then Commit |
| ------------------ |
| |
| Code modifications must go through a link:http://www.apache.org/foundation/glossary.html#ReviewThenCommit[review-then-committ] (RTC) |
| process before being merged into a release branch. All committers should follow the pattern below, where "you" refers to the |
| committer wanting to put code into a release branch. |
| |
| * Make a JIRA ticket for the software problem you want to solve (i.e. a fix). |
| * Fork the release branch that the fix will be put into. |
| ** The branch name should be the JIRA issue identifier (e.g. `TINKERPOP-XXX`). |
| * Develop your fix in your branch. |
| * When your fix is complete and ready to merge, issue a link:https://git-scm.com/docs/git-request-pull[pull request]. |
| ** Be certain that the test suite is passing. |
| ** If you updated documentation, be sure that the `process-docs.sh` is building the documentation correctly. |
| * Before you can merge your branch into the release branch, you must have at least 3 +1 link:http://www.apache.org/foundation/glossary.html#ConsensusApproval[consensus votes]. |
| ** Please see the Apache Software Foundations regulations regarding link:http://www.apache.org/foundation/voting.html#votes-on-code-modification[Voting on Code Modifications]. |
| * Votes are issued by TinkerPop committers as comments to the pull request. |
| * Once 3 +1 votes are received, you are responsible for merging to the release branch and handling any merge conflicts. |
| ** If there is a higher version release branch that requires your fix (e.g. `3.y-1.z` fix going to a `3.y.z` release), be sure to merge to that release branch as well. |
| * Be conscious of deleting your branch if it is no longer going to be used so stale branches don't pollute the repository. |
| |
| NOTE: These steps also generally apply to external pull requests from those who are not official Apache committers. In |
| this case, the person responsible for the merge after voting is typically the first person available |
| who is knowledgeable in the area that the pull request affects. Any additional coordination on merging can be handled |
| via the pull request comment system. |
| |
| The following exceptions to the RTC (review-then-commit) model presented above are itemized below. It is up to the |
| committer to self-regulate as the itemization below is not complete and only hints at the types of commits that do not |
| require a review. |
| |
| * You are responsible for a release and need to manipulate files accordingly for the release. |
| ** `Gremlin.version()`, CHANGELOG dates, `pom.xml` version bumps, etc. |
| * You are doing an minor change and it is obvious that an RTC is not required (would be a pointless burden to the community). |
| ** The fix is under the link:http://www.apache.org/foundation/glossary.html#CommitThenReview[commit-then-review] (CTR) policy and lazy consensus is sufficient, where a single -1 vote requires you to revert your changes. |
| ** Adding a test case, fixing spelling/grammar mistakes in the documentation, fixing LICENSE/NOTICE/etc. files, fixing a minor issue in an already merged branch. |
| |
| When the committer chooses CTR, it is considered good form to include something in the commit message that explains |
| that CTR was invoked and the reason for doing so. For example, "Invoking CTR as this change encompasses minor |
| adjustments to text formatting." |
| |
| Pull Request Format |
| ~~~~~~~~~~~~~~~~~~~ |
| |
| When you submit a pull request, be sure it uses the following style. |
| |
| * The title of the pull request is the JIRA ticket number + "colon" + the title of the JIRA ticket. |
| * The first line of the pull request message should contain a link to the JIRA ticket. |
| * Discuss what you did to solve the problem articulated in the JIRA ticket. |
| * Discuss any "extra" work done that go beyond the assumed requirements of the JIRA ticket. |
| * Be sure to explain what you did to prove that the issue is resolved. |
| ** Test cases written. |
| ** Integration tests run (if required for the work accomplished). |
| ** Documentation building (if required for the work accomplished). |
| ** Any manual testing (though this should be embodied in a test case). |
| * Notes about what you will do when you merge to the respective release branch (e.g. update CHANGELOG). |
| ** These types of "on merge tweaks" are typically done to extremely dynamic files to combat and merge conflicts. |
| * If you are a TinkerPop committer, you can VOTE on your own pull request, so please do so. |
| |
| [[dependencies]] |
| Dependencies |
| ------------ |
| |
| There are many dependencies on other open source libraries in TinkerPop modules. When adding dependencies or |
| altering the version of a dependency, developers must consider the implications that may apply to the TinkerPop |
| LICENSE and NOTICE files. There are two implications to consider: |
| |
| . Does the dependency fit an Apache _approved_ license? |
| . Given the addition or modification to a dependency, does it mean any change for TinkerPop LICENSE and NOTICE files? |
| |
| Understanding these implications is important for insuring that TinkerPop stays compliant with the Apache 2 license |
| that it releases under. |
| |
| Regarding the first item, refer to the Apache Legal for a list of link:http://www.apache.org/legal/resolved.html[approved licenses] |
| that are compatible with the Apache 2 license. |
| |
| The second item requires a bit more effort to follow. The Apache website offers a |
| link:http://www.apache.org/dev/licensing-howto.html[how-to guide] on the approach to maintaining appropriate LICENSE |
| and NOTICE files, but this guide is designed to offer some more specific guidance as it pertains to TinkerPop |
| and its distribution. |
| |
| To get started, TinkerPop has both "source" and "binary" LICENSE/NOTICE files: |
| |
| * Source LICENSE/NOTICE relate to files packaged with the released source code distribution: |
| link:https://github.com/apache/tinkerpop/blob/master/LICENSE[LICENSE] / link:https://github.com/apache/tinkerpop/blob/master/NOTICE[NOTICE] |
| * Binary LICENSE/NOTICE relate to files packaged with the released binary distributions: |
| ** Gremlin Console link:https://github.com/apache/tinkerpop/blob/master/gremlin-console/src/main/LICENSE[LICENSE] |
| / link:https://github.com/apache/tinkerpop/blob/master/gremlin-console/src/main/NOTICE[NOTICE] |
| ** Gremlin Server link:https://github.com/apache/tinkerpop/blob/master/gremlin-server/src/main/LICENSE[LICENSE] |
| / link:https://github.com/apache/tinkerpop/blob/master/gremlin-server/src/main/NOTICE[NOTICE] |
| |
| Source LICENSE and NOTICE |
| ~~~~~~~~~~~~~~~~~~~~~~~~~ |
| |
| As dependencies are not typically added to the source distribution (i.e. the source zip distribution), there is |
| typically no need to edit source LICENSE/NOTICE when editing a TinkerPop `pom.xml`. These files only need to be edited |
| if the distribution has a file added to it. Such a situation may arise from several scenarios, but it would most |
| likely come from the addition of a source file from another library. |
| |
| * If the file being bundled is Apache licensed, then add an entry to NOTICE. |
| * If the file being bundled is under a different approved license, then add an entry to LICENSE and include a copy of |
| that LICENSE in the root `/licenses` directory of the code repository. |
| |
| Binary LICENSE and NOTICE |
| ~~~~~~~~~~~~~~~~~~~~~~~~~ |
| |
| The binary LICENSE/NOTICE is perhaps most impacted by changes to the various `pom.xml` files. After altering the |
| `pom.xml` file of any module, build both Gremlin Console and Gremlin Server and examine the contents of both binary |
| distributions, either: |
| |
| * target/apache-gremlin-console-x.y.z-distribution.zip |
| * target/apache-gremlin-server-x.y.z-distribution.zip |
| |
| Apache licensed software does not need to be included in LICENSE, but if the new dependency is an Apache-approved |
| license (e.g. BSD, MIT) then it should be added in the pattern already defined. A copy of the LICENSE should be |
| added to the `<project>/static/licenses` directory of the code repository. |
| |
| To determine if changes are required to the NOTICE, first check if the bundled jar has a NOTICE file in it (typically |
| found in `/META-INF` directory of the jar). |
| |
| * If the bundled file does not have a NOTICE, then no changes to TinkerPop's NOTICE are required. |
| * If the NOTICE of the file being bundled is NOT Apache licensed, then there is no change to TinkerPop's NOTICE. |
| * If the NOTICE of the file being bundled is Apache licensed, then include the copyright notification in TinkerPop's |
| NOTICE. |
| * If the NOTICE of the file being bundled is Apache licensed AND is an Apache Software Foundation project, then |
| ONLY include the portion of that NOTICE in TinkerPop's NOTICE that is unrelated to the Apache boilerplate NOTICE. |
| If there is no such portion that is different than the boilerplate then this NOTICE can be excluded (i.e. don't |
| alter TinkerPop's NOTICE at all). |
| |
| Please refer to the link:http://www.apache.org/dev/licensing-howto.html#mod-notice[Modifications to Notice] section |
| of the Apache "Licensing How-to" for more information. |
| |
| [[documentation]] |
| Documentation |
| ------------- |
| |
| The documentation for TinkerPop is stored in the git repository in `docs/src/` and are then split into several |
| subdirectories, each representing a "book" (or its own publishable body of work). If a new AsciiDoc file is added to |
| a book, then it should also be included in the `index.asciidoc` file for that book, otherwise the preprocessor will |
| ignore it. Likewise, if a whole new book (subdirectory) is added, it must include an `index.asciidoc` file to be |
| recognized by the AsciiDoc preprocessor. |
| |
| Adding a book also requires a change to the root `pom.xml` file. Find the "asciidoc" Maven profile and add a new |
| `<execution>` to the `asciidoctor-maven-plugin` configuration. For each book in `docs/src/`, there should be a |
| related `<execution>` that generates the HTML from the AsciiDoc. Follows the patterns already established by |
| the existing `<execution>` entries, paying special attention to the pathing of the '<sourceDirectory>', |
| `<outputDirectory>` and `<imagesdir>`. Note that the `<outputDirectory>` represents where the book will exist when |
| uploaded to the server and should preserve the directory structure in git as referenced in `<sourceDirectory>`. |
| |
| Please see the <<building-testing,Building and Testing>> section for more information on how to generate the |
| documentation. |
| |
| [[logging]] |
| Logging |
| ------- |
| |
| TinkerPop uses SLF4j for logging and typically leans back on Log4j as the implementation. Configuring log outputs |
| for debugging purposes within tests can be altered by editing the `log4j-test.properties` file in each module's test |
| resources. That file gets copied to the `target/test-classes` on build and surefire and failsafe plugins in maven |
| are then configured to point at that area of the file system for those configuration files. The properties files |
| can be edited to fine tune control of the log output, but generally speaking the current configuration is likely |
| best for everyone's general purposes, so if changes are made please revert them prior to commit. |