Keystone: Changeset commits can be easily (and silently) dropped when rebasing

Created on 21 Sep 2018  路  3Comments  路  Source: keystonejs/keystone

According to CONTRIBUTING.md:

IMPORTANT INFORMATION By default, changesets generate empty git commits, unless you had any staged changes. If you have empty commits, a rebase removes them by default, so changesets that are empty will be lost.

This is concerning as rebasing (or squash & merge) is a very common workflow both via GitHub, and during local development. Imagine adding a major changeset, only to have it published as a minor or patch because it was rebased away and subsequent changesets were added.

I would prefer to make this much much harder to occur.

One possible solution:

Have a .changeset file with the contents:

# This file is used to generate non-empty commits for changeses. Do not edit manually.
# The answer the great question of Life, The Universe, And Everything is:
1

Where the 1 is modified everytime a changeset is generated.

TBA bug docs

Most helpful comment

To raise this up a level, I'd say the concern here is

The change tracking tooling disallows git changes that are destructive to git history - even when those changes are within a local branch

This restriction exists because we are using git history to store data.

The reasons we use git history:

  • It's always part of the repo
  • It connects where the changes occurred with the changes to release, so changelogs can have links back to the commits
  • It doesn't require the user committing additional context

After talking to Jess and Tim offline, we want to to try a different approach that should mostly just work (TM) with the same tools. This is to use the file-system as the database using the following structure:

- .changeset
  - unique-hashed-folder
    - changes.md
    - changes.json

Some notes on this structure:

  • Because of how git history works, it is vital that these folders have unique names. Writing to a single file means all pull requests are immediately in conflict.
  • Because git history is destructible, we cannot rely on when changesets were added to a repo to track whether they should be included in a release, or what commit to link to in the changelog.

The first can be worked around with unique file (or folder) names.

The changes to the release steps would be as follows:

yarn changeset

  • Same question/answer.
  • Instead of writing to git history, generate the .changeset folder if it is missing, generate a unique-id-ed folder, and the two files to contain changeset information

(A quick aside, the two files are so the md part of the changeset is a markdown file for ease-of-editing, while the rest remains as json data)

yarn version-packages

  • This will generate a release commit as before, using ALL changesets in the .changeset folder when the command is run
  • Use the git hash where the changeset files were added as the canonical source of the changes
  • This will delete ALL changesets in the .changeset folder as part of the release commit

yarn publish-packages

  • no change

Other benefits:

  • Changesets are easier to modify after-the-fact
  • Manually creating changesets becomes possible
  • changelog files could manually be given meaningful names (though these would never be used)

Was originally planning on just forking the releases packages to do a trial run, but talked it over with Luke and we are likely going to just make this change in the main releases package.

All 3 comments

To raise this up a level, I'd say the concern here is

The change tracking tooling disallows git changes that are destructive to git history - even when those changes are within a local branch

This restriction exists because we are using git history to store data.

The reasons we use git history:

  • It's always part of the repo
  • It connects where the changes occurred with the changes to release, so changelogs can have links back to the commits
  • It doesn't require the user committing additional context

After talking to Jess and Tim offline, we want to to try a different approach that should mostly just work (TM) with the same tools. This is to use the file-system as the database using the following structure:

- .changeset
  - unique-hashed-folder
    - changes.md
    - changes.json

Some notes on this structure:

  • Because of how git history works, it is vital that these folders have unique names. Writing to a single file means all pull requests are immediately in conflict.
  • Because git history is destructible, we cannot rely on when changesets were added to a repo to track whether they should be included in a release, or what commit to link to in the changelog.

The first can be worked around with unique file (or folder) names.

The changes to the release steps would be as follows:

yarn changeset

  • Same question/answer.
  • Instead of writing to git history, generate the .changeset folder if it is missing, generate a unique-id-ed folder, and the two files to contain changeset information

(A quick aside, the two files are so the md part of the changeset is a markdown file for ease-of-editing, while the rest remains as json data)

yarn version-packages

  • This will generate a release commit as before, using ALL changesets in the .changeset folder when the command is run
  • Use the git hash where the changeset files were added as the canonical source of the changes
  • This will delete ALL changesets in the .changeset folder as part of the release commit

yarn publish-packages

  • no change

Other benefits:

  • Changesets are easier to modify after-the-fact
  • Manually creating changesets becomes possible
  • changelog files could manually be given meaningful names (though these would never be used)

Was originally planning on just forking the releases packages to do a trial run, but talked it over with Luke and we are likely going to just make this change in the main releases package.

Just an aside, we would want to still make sure not to run destructive git operations on release commits, otherwise you can end up in very odd states. Since workflow is that these should be immediately merged into master, that seems fine.

@Noviny I believe this issue can now be closed? If so, can you close it, or else let us know why it should be left open.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

justinmoon picture justinmoon  路  13Comments

molomby picture molomby  路  11Comments

bpavot picture bpavot  路  11Comments

wesbos picture wesbos  路  16Comments

jesstelford picture jesstelford  路  14Comments