<commit_before>---
authors: <NAME> <<EMAIL>>
state: draft
discussion: https://github.com/joyent/rfd/issues/143
---

<!--
    This Source Code Form is subject to the terms of the Mozilla Public
    License, v. 2.0. If a copy of the MPL was not distributed with this
    file, You can obtain one at http://mozilla.org/MPL/2.0/.
-->

<!--
    Copyright 2019 Joyent, Inc.
-->

# RFD 175 SmartOS integration process changes

## Problem

The vast majority of Joyent repositories have now been moved out of the Joyent
Gerrit server, leaving basically just the "platform" repositories, namely:

1. smartos-live
1. illumos-joyent
1. illumos-extra
1. illumos-kvm
1. illumos-kvm-cmd

With little appetite for continuing to maintain our own Gerrit server solely for
the platform repositories, we need to look at other options, and settle on a
plan that causes least friction for making platform changes easily. We'll
presume from this point that [cr.joyent.us](https://cr.joyent.us/) is no longer
going to be available.

## Github pull requests

The other Joyent repositories all now use github PRs for their integration
process. While some automation in this area is still in progress, this appears
to be working well in general.

However, for illumos-joyent especially, PRs can be a pretty miserable experience
when it comes to reviewing larger changes. In particular, the diff browser
insists on every file's diff being on the same page, making it close to
impossible to keep track of a large review, or newly introduced large files. The
canonical example I've been using is
[KPTI](https://github.com/illumos/illumos-gate/commit/74ecdb5171c9f3673b9393b1a3dc6f3a65e93895#diff-9ddb7d82a1170d4cf11ae141b03511b6).

It's common to pull down the PR to a local checkout for large reviews, but this
is still unpleasant when trying to comment on the actual diffs in the PR.

This is likely to be much less of an issue outside of illumos-joyent: large changes
are much rarer, although they can happen in the case of smartos-live.

The specific proposal here is to use github PRs for all of these repositories,
despite the drawbacks. For significant changes that prove difficult to review
via github, reviewers can request a webrev. In this case, review comments should
be added to the PR on the comment thread - it's not necessary to add them in
place in the PR's diffs, as that would defeat the point. If feasible though,
reviews should be done in github.

### Commit format

After we make this change, commit messages look like this:

```
OS-99999 Really kill Perl

OS-100000 Also Python
Reviewed by: <NAME> <<EMAIL>>
Approved by: <NAME> <<EMAIL>>
```

As previously, no free-form text is allowed or expected: prose should be in the ticket,
perhaps in the "Description" so it's clearly visible.

Note the addition of the git-style blank line: this change is for multiple reasons, but
the most relevant is that you cannot avoid adding this using the github user interface.
We don't want to *require* use of additional tooling.

Any additional tickets should go on the third line next to the tags. Commits should not
needlessly group unrelated tickets together.

No changes are planned to what these annotations currently mean. Bugs will still be
filed and managed in JIRA. We would like to keep the requirement of one code review
AND one integration approval before a PR can be merged.

The hope is that this is managed by labels on the PR, and merge will be blocked until
at least one of each label is seen. Tooling is being worked on to automate this commit
message format at merge time, in a similar fashion to how
[grr](https://github.com/joyent/grr) worked.

The process will look something like this for repositories other than illumos-joyent:

1. develop your changes on a branch
1. file JIRA bug(s) as before with more details on the changes
1. open a PR against master, pointing to JIRA as needed.
1. request reviewers, wait for code review (as represented by a label)
1. iterate as needed
1. add testing notes on any final version
1. wait for integration approval (as represented by a label)
1. squash and merge your change, with the commit message in the form above
1. close the JIRA(s) as Fixed

## Upstream first of illumos changes

One major change we'd like to experiment with concerns changes to illumos-joyent
that apply in part or in whole to illumos-gate.

Historically, we'd integrate a fix into illumos-joyent, and then sometime later,
an engineer might port the changes over to illumos-gate, build and re-verify as
needed, then go through the [standard illumos-gate RTI
process](https://illumos.org/books/dev/integrating.html). Soon after, those
changes would be merged *back* up to illumos-joyent as part of the daily merge.

This is clearly not working very well, as can be seen by [comparing our
trees](https://us-east.manta.joyent.com/Joyent_Dev/public/webrevs/platform-upstream-webrev/index.html).
It involves an unpleasant amount of busy-work resolving conflicts in both<commit_msg>Update RFD 175 SmartOS integration process changes<commit_after>---
authors: <NAME> <<EMAIL>>
state: draft
discussion: https://github.com/joyent/rfd/issues/143
---

<!--
    This Source Code Form is subject to the terms of the Mozilla Public
    License, v. 2.0. If a copy of the MPL was not distributed with this
    file, You can obtain one at http://mozilla.org/MPL/2.0/.
-->

<!--
    Copyright 2019 Joyent, Inc.
-->

# RFD 175 SmartOS integration process changes

## Problem

The vast majority of Joyent repositories have now been moved out of the Joyent
Gerrit server, leaving basically just the "platform" repositories, namely:

1. smartos-live
1. illumos-joyent
1. illumos-extra
1. illumos-kvm
1. illumos-kvm-cmd

With little appetite for continuing to maintain our own Gerrit server solely for
the platform repositories, we need to look at other options, and settle on a
plan that causes least friction for making platform changes easily. We'll
presume from this point that [cr.joyent.us](https://cr.joyent.us/) is no longer
going to be available.

## Github pull requests

The other Joyent repositories all now use github PRs for their integration
process. While some automation in this area is still in progress, this appears
to be working well in general.

However, for illumos-joyent especially, PRs can be a pretty miserable experience
when it comes to reviewing larger changes. In particular, the diff browser
insists on every file's diff being on the same page, making it close to
impossible to keep track of a large review, or newly introduced large files. The
canonical example I've been using is
[KPTI](https://github.com/illumos/illumos-gate/commit/74ecdb5171c9f3673b9393b1a3dc6f3a65e93895#diff-9ddb7d82a1170d4cf11ae141b03511b6).

It's common to pull down the PR to a local checkout for large reviews, but this
is still unpleasant when trying to comment on the actual diffs in the PR.

This is likely to be much less of an issue outside of illumos-joyent: large changes
are much rarer, although they can happen in the case of smartos-live.

The specific proposal here is to use github PRs for all of these repositories,
despite the drawbacks. For significant changes that prove difficult to review
via github, reviewers can request a webrev. In this case, review comments should
be added to the PR on the comment thread - it's not necessary to add them in
place in the PR's diffs, as that would defeat the point. If feasible though,
reviews should be done in github.

### Commit format

After we make this change, commit messages look like this:

```
OS-99999 Really kill Perl (#223)

OS-100000 Also Python
Reviewed by: <NAME> <<EMAIL>>
Approved by: <NAME> <<EMAIL>>
```

As previously, no free-form text is allowed or expected: prose should be in the ticket,
perhaps in the "Description" so it's clearly visible.

Note the addition of the git-style blank line: this change is for multiple reasons, but
the most relevant is that you cannot avoid adding this using the github user interface.
We don't want to *require* use of additional tooling.

The oneline also includes the pull request number (#223), as this is automatically added
by github, and it seems useful to have a direct link to the PR.

Any additional tickets should go on the third line next to the tags. Commits should not
needlessly group unrelated tickets together.

No changes are planned to what these annotations currently mean. Bugs will still be
filed and managed in JIRA. There is still an ongoing requirement of one code review
AND one integration approval before a PR can be merged to the master branch.

Intergration approval is managed by a label on the PR, and merge will be blocked until
one is added along with at least one "approval" (code review).
Tooling is being worked on to automate this commit message format at merge time, in a
similar fashion to how [grr](https://github.com/joyent/grr) worked.

The process will look something like this for repositories other than illumos-joyent:

1. develop your changes on a branch
1. file JIRA bug(s) as before with more details on the changes
1. open a PR against master, pointing to JIRA as needed.
1. request reviewers, wait for code review (as represented by a label)
1. iterate as needed
1. add testing notes on any final version
1. wait for integration approval (as represented by a label)
1. squash and merge your change, with the commit message in the form above
1. close the JIRA(s) as Fixed

## Upstream first of illumos changes

One major change we'd like to experiment with concerns changes to illumos-joyent
that apply in part or in whole to illumos-gate.

Historically, we'd integrate a fix into illumos-joyent, and then sometime later,
an engineer might port the changes over to illumos-gate, build and re-verify as
needed, then go through the [standard illumos-gate RTI
process](https://illumos.org/books/dev/integrating.html). Soon after, those
changes would be merged *back* up to illumos-joyent as part of the daily merge.

This is clearly not working very well, as can be seen by [comparing our
trees](https://us-east.manta.joyent.com/Joyent_Dev/public/webrevs/platform-upstream-webrev/index.html).
It involves an unpleasant amount of busy-work resolving conflicts in both
{'fraction_non_alphanumeric': 0.05678819274753201, 'fraction_numerical': 0.01749584595836184, 'mean_word_length': 4.257965056526207, 'pattern_counts': {'":': 0, '<': 25, '<?xml version=': 0, '>': 25, 'https://': 12, 'lorem ipsum': 0, 'www.': 0, 'xml': 0}, 'pii_count': 0, 'substrings_counts': 0, 'word_list_counts': {'cursed_substrings.json': 0, 'profanity_word_list.json': 0, 'sexual_word_list.json': 0, 'zh_pornsignals.json': 0}}
{'dir': 'git-commits-cleaned', 'id': '2803586', 'n_tokens_mistral': 2961, 'n_tokens_neox': 2719, 'n_words': 1552}