Skip to content

Add release process section to DEVELOPMENT.md #14020

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

Merged
electrum merged 2 commits into from
Feb 6, 2023
Merged

Add release process section to DEVELOPMENT.md #14020

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
927ddec
Fix wrapping in DEVELOPMENT.md
colebow Sep 12, 2022
a1dae55
Add release section to DEVELOPMENT.md
colebow Sep 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
129 changes: 90 additions & 39 deletions .github/DEVELOPMENT.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,26 @@ When writing a Git commit message, follow these [guidelines](https://chris.beams

Pull requests are usually merged into `master` using the [`rebase and merge`](https://docs.github.com/en/github/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/about-pull-request-merges#rebase-and-merge-your-pull-request-commits) strategy.

A typical pull request should strive to contain a single logical change (but not necessarily a single commit).
Unrelated changes should generally be extracted into their own PRs.

If a pull request does consist of multiple commits, it is expected that every prefix of it is correct. That is,
there might be preparatory commits at the bottom of the stack that don't bring any value by themselves,
but none of the commits should introduce an error that is fixed by some future commit.
Every commit should build and pass all tests.

Commit messages and history are also important, as they are used by other developers to keep track of
the motivation behind changes. Keep logical diffs grouped together in separate commits, and order commits
in a way that explains the progress of the changes. Rewriting and reordering commits may be a
necessary part of the PR review process as the code changes. Mechanical changes (like refactoring and renaming)
should be separated from logical and functional changes. E.g. deduplicating code or extracting helper methods should happen
in a separate commit from the commit where new features or behavior is introduced. This makes reviewing the code
much easier and reduces the chance of introducing unintended changes in behavior.
A typical pull request should strive to contain a single logical change (but not
necessarily a single commit). Unrelated changes should generally be extracted
into their own PRs.

If a pull request does consist of multiple commits, it is expected that every
prefix of it is correct. That is, there might be preparatory commits at the
bottom of the stack that don't bring any value by themselves, but none of the
commits should introduce an error that is fixed by some future commit. Every
commit should build and pass all tests.

Commit messages and history are also important, as they are used by other
developers to keep track of the motivation behind changes. Keep logical diffs
grouped together in separate commits, and order commits in a way that explains
the progress of the changes. Rewriting and reordering commits may be a necessary
part of the PR review process as the code changes. Mechanical changes (like
refactoring and renaming)should be separated from logical and functional
changes. E.g. deduplicating code or extracting helper methods should happen in a
separate commit from the commit where new features or behavior is introduced.
This makes reviewing the code much easier and reduces the chance of introducing
unintended changes in behavior.

## Code Style

Expand All @@ -49,10 +54,11 @@ In addition to those you should also adhere to the following:

### Readability

The purpose of code style rules is to maintain code readability and developer efficiency when
working with the code. All the code style rules explained below are good guidelines to follow
but there may be exceptional situations where we purposefully depart from them. When readability
and code style rule are at odds, the readability is more important.
The purpose of code style rules is to maintain code readability and developer
efficiency when working with the code. All the code style rules explained below
are good guidelines to follow but there may be exceptional situations where we
purposefully depart from them. When readability and code style rule are at odds,
the readability is more important.

### Consistency

Expand Down Expand Up @@ -138,20 +144,22 @@ the code base like `ttl` are allowed and encouraged.

### Avoid default clause in exhaustive enum-based switch statements

Avoid using the `default` clause when the switch statement is meant to cover all the
enum values. Handling the unknown option case after the switch statement allows static code
analysis tools (e.g. Error Prone's `MissingCasesInEnumSwitch` check) report a problem
when the enum definition is updated but the code using it is not.
Avoid using the `default` clause when the switch statement is meant to cover all
the enum values. Handling the unknown option case after the switch statement
allows static code analysis tools (e.g. Error Prone's `MissingCasesInEnumSwitch`
check) report a problem when the enum definition is updated but the code using
it is not.

## Additional IDE configuration

When using IntelliJ to develop Trino, we recommend starting with all of the default inspections,
with some modifications.
When using IntelliJ to develop Trino, we recommend starting with all of the
default inspections, with some modifications.

Enable the following inspections:

- ``Java | Internationalization | Implicit platform default charset``,
- ``Java | Control flow issues | Redundant 'else'`` (including ``Report when there are no more statements after the 'if' statement`` option),
- ``Java | Control flow issues | Redundant 'else'`` (including
``Report when there are no more statements after the 'if' statement`` option),
- ``Java | Class structure | Utility class is not 'final'``,
- ``Java | Class structure | Utility class with 'public' constructor``,
- ``Java | Class structure | Utility class without 'private' constructor``.
Expand All @@ -170,41 +178,84 @@ Enable errorprone ([Error Prone Installation#IDEA](https://errorprone.info/docs/
- Install ``Error Prone Compiler`` plugin from marketplace,
- Check the `errorprone-compiler` profile in the Maven tab

This should be enough - IDEA should automatically copy the compiler options from the POMs to each module. If that doesn't work, you can do it manually:
This should be enough - IDEA should automatically copy the compiler options from
the POMs to each module. If that doesn't work, you can do it manually:

- In ``Java Compiler`` tab, select ``Javac with error-prone`` as the compiler,
- Update ``Additional command line parameters`` and copy the contents of ``compilerArgs`` in the top-level POM (except for ``-Xplugin:ErrorProne``) there
- Update ``Additional command line parameters`` and copy the contents of
``compilerArgs`` in the top-level POM (except for ``-Xplugin:ErrorProne``)
there
- Remove the XML comments...
- ...except the ones which denote checks which fail in IDEA, which you should "unwrap"
- ...except the ones which denote checks which fail in IDEA, which you should
"unwrap"
- Remove everything from the list under ``Override compiler parameters per-module``

Note that the version of errorprone used by the IDEA plugin might be older than the one configured in the `pom.xml` and you might need to disable some checks that are not yet supported by that older version. When in doubt, always check with the full Maven build (``./mvnw clean install -DskipTests -Perrorprone-compiler``).
Note that the version of errorprone used by the IDEA plugin might be older than
the one configured in the `pom.xml` and you might need to disable some checks
that are not yet supported by that older version. When in doubt, always check
with the full Maven build (``./mvnw clean install -DskipTests -Perrorprone-compiler``).

### Language injection in IDE

In order to enable language injection inside Intellij IDEA, some code elements can be annotated with the `@org.intellij.lang.annotations.Language` annotation. To make it useful, we recommend:
In order to enable language injection inside Intellij IDEA, some code elements
can be annotated with the `@org.intellij.lang.annotations.Language` annotation.
To make it useful, we recommend:

- Set the project-wide SQL dialect in ``Languages & Frameworks | SQL Dialects`` - "Generic SQL" is a decent choice here,
- Set the project-wide SQL dialect in ``Languages & Frameworks | SQL Dialects``
"Generic SQL" is a decent choice here,
- Disable inspection ``SQL | No data source configured``,
- Optionally disable inspection ``Language injection | Language mismatch``.

Even if the IDE does not support language injection this annotation is useful for documenting the API's intent. Considering the above, we recommend annotating with `@Language`:
Even if the IDE does not support language injection this annotation is useful
for documenting the API's intent. Considering the above, we recommend annotating
with `@Language`:

- All API parameters which are expecting to take a `String` containing a statement written in SQL (or any other language, like regular expressions),
- Local variables which otherwise would not be properly recognized by IDE for language injection.
- All API parameters which are expecting to take a `String` containing an SQL
statement (or any other language, like regular expressions),
- Local variables which otherwise would not be properly recognized by IDE for
language injection.

## Building the Web UI

The Trino Web UI is composed of several React components and is written in JSX and ES6. This source code is compiled and packaged into browser-compatible Javascript, which is then checked in to the Trino source code (in the `dist` folder). You must have [Node.js](https://nodejs.org/en/download/) and [Yarn](https://yarnpkg.com/en/) installed to execute these commands. To update this folder after making changes, simply run:
The Trino Web UI is composed of several React components and is written in JSX
and ES6. This source code is compiled and packaged into browser-compatible
Javascript, which is then checked in to the Trino source code (in the `dist`
folder). You must have [Node.js](https://nodejs.org/en/download/) and
[Yarn](https://yarnpkg.com/en/) installed to execute these commands. To update
this folder after making changes, simply run:

yarn --cwd core/trino-main/src/main/resources/webapp/src install

If no Javascript dependencies have changed (i.e., no changes to `package.json`), it is faster to run:
If no Javascript dependencies have changed (i.e., no changes to `package.json`),
it is faster to run:

yarn --cwd core/trino-main/src/main/resources/webapp/src run package

To simplify iteration, you can also run in `watch` mode, which automatically re-compiles when changes to source files are detected:
To simplify iteration, you can also run in `watch` mode, which automatically
re-compiles when changes to source files are detected:

yarn --cwd core/trino-main/src/main/resources/webapp/src run watch

To iterate quickly, simply re-build the project in IntelliJ after packaging is complete. Project resources will be hot-reloaded and changes are reflected on browser refresh.
To iterate quickly, simply re-build the project in IntelliJ after packaging is
complete. Project resources will be hot-reloaded and changes are reflected on
browser refresh.

## Releases

Trino aims for frequent releases, generally once per week. This is a goal but
not a guarantee, as critical bugs may lead to a release being pushed back or
require an extra emergency release to patch the issue.

At the start of each release cycle, a GitHub issue is filed and pinned to track
all necessary release notes. For example, see [the issue for Trino 395](https://github.com/trinodb/trino/issues/13913).
In addition, a release notes pull request is updated and maintained throughout
the week, tracking all merged commits to ensure every change is properly
documented and noted. This uses the [release note template](../docs/release-template.md),
with changes in each section arranged to have new features first, performance
improvements second, and bugfixes third. See [the release notes for 395](https://github.com/trinodb/trino/pull/13975)
as an example.

Once it is time to release, the release process is kicked off. A code freeze is
announced on the Trino Slack in the #releases channel, and then a maintainer
utilizes the [release scripts](https://github.com/trinodb/release-scripts) to
update Trino to the next version.