Add squash merges option for editorial workflow #1330

delucis · 2018-04-29T18:06:28Z

Summary

Exposes a backend.squash_merges option, which allows users to configure the merge method used when merging Github pull requests. The standard behaviour remains unchanged, but setting squash_merges: true will use the squash merge method instead (see Github API docs).

Why?

When using the Editorial Workflow, every small change generates at least two commits in the repository history with standard merges, more if content was edited several times before publishing. For content-heavy sites, this can mean the commit history is overloaded with (pretty uninformative) merge commits. Squash merges allow users to minimise clutter in the commit history if they want to.

Considerations

I’m not sure how this fits together with planning for other backends.

Test plan

npm run test passed
npm run lint threw 281935 problems (245884 errors, 36051 warnings), which I’m hoping my changes didn’t cause 😂
Tested in a branch of a Nuxt + Netlify CMS site deployed to Netlify (using git-gateway) and seems to be working, would be happy to follow up suggestions for further testing

Description for the changelog

Add squash merges option for editorial workflow

A picture of a cute animal (not mandatory but encouraged)

An okapi 😀

verythorough · 2018-04-29T18:08:15Z

Deploy preview for netlify-cms-www ready!

Built with commit b644032

https://deploy-preview-1330--netlify-cms-www.netlify.com

verythorough · 2018-04-29T18:08:27Z

Deploy preview for cms-demo ready!

Built with commit c6ab176

https://deploy-preview-1330--cms-demo.netlify.com

verythorough · 2018-04-29T18:10:16Z

Deploy preview for cms-demo ready!

Built with commit b644032

https://deploy-preview-1330--cms-demo.netlify.com

Benaiah · 2018-05-02T01:03:07Z

@delucis re: fitting in with other backends: part of the backend API discussion has been around separating options that are specific to a given backend from options that would apply to any backend used by the CMS¹. This is currently unimplemented, so I think the current location is fine, but I think we may want to document this as a preview feature so that we're not committed to maintaining the current key location when it may change soon.

So basically, there are some considerations around other backends, but I don't think there's any related changes needed in this PR before it's merged.

¹ e.g., something like the following:

backend:
  name: github
  settings:
    squash_merges: true # this setting is github-specific

It probably won't look exactly like this, but that's the general idea.

delucis · 2018-05-02T09:49:02Z

@Benaiah Thanks! If this should be documented as a “preview feature”, should I document it on the “Beta Features!” page? Or just add a note where it is at the moment?

Out of curiosity, just looked into how squash merging would work with the BitBucket and GitLab APIs, so posting that here for future reference:

BitBucket pull requests can be squash merged by passing merge_strategy: squash when merging [Docs →]
GitLab merge requests have a boolean squash property that is set directly on the request (at create/update) rather than when the request is merged [Docs →]

Benaiah · 2018-05-02T21:16:58Z

@delucis I think the Beta Features page would be the best spot (@erquhart @verythorough what do you think?). Thanks for looking into GitLab and BitBucket! It looks like it should be straightforward to support this feature on those backends as well in the future.

delucis · 2018-05-03T09:42:53Z

OK! Moved the documentation to the Beta Features page: https://deploy-preview-1330--netlify-cms-www.netlify.com/docs/beta-features/#squash-merge-github-pull-requests

verythorough

Such a great feature addition! I added one comment on the doc.

verythorough · 2018-05-04T23:12:51Z

website/site/content/docs/beta-features.md

@@ -77,3 +77,10 @@ import styles from '!css-loader!sass-loader!../main.scss'
 CMS.registerPreviewStyle(styles.toString(), { raw: true })
 ```

+## Squash merge GitHub pull requests
+When using the [Editorial Workflow publish mode](/docs/configuration-options/#publish-mode) with the `github` or `git-gateway` backends, Netlify CMS creates pull requests to hold drafts while they are being edited and then merges them into your main branch when you press publish. By default, these are merged preserving the history of all the individual commits (i.e. changes where you pressed save). If instead you would like [“squash”](https://help.github.com/articles/about-pull-request-merges/#squash-and-merge-your-pull-request-commits) these commits, reducing clutter in your repository’s history, you can set the following option in your `config.yml`:


Great description! The one suggestion I would make is to simplify the second-to-last sentence. Parentheticals and Latin abbreviations like i.e. can be difficult for translators, etc. I suggest ending the sentence after "individual commits" and changing the parenthetical to a sentence, like:

In other words, you'll have a commit in your history for every time you pressed the save button.

I would also suggest adding a paragraph break before the last sentence ("If instead...").

delucis · 2018-05-05T04:54:46Z

@verythorough Thanks for the review! Definitely an improvement: https://deploy-preview-1330--netlify-cms-www.netlify.com/docs/beta-features/#squash-merge-github-pull-requests

delucis · 2018-05-11T15:30:53Z

@Benaiah @verythorough Let me know if there’s anything I can do to improve this before it’s merged!

erquhart

Looks great! Just one change request and this is good to merge.

erquhart · 2018-05-12T16:13:18Z

src/actions/config.js

@@ -50,6 +50,9 @@ export function validateConfig(config) {
  if (typeof config.getIn(['backend', 'name']) !== 'string') {
    throw new Error("Error in configuration file: Your `backend.name` must be a string. Check your config.yml file.");
  }
+  if (!isBoolean(config.getIn(['backend', 'squash_merges'], false))) {


Leave this out, trying to avoid validation of beta features, plus this is specific to one backend. After 2.0 I expect we'll provide a way for specific backends to validate their configuration, as a similar effort is underway for widgets.

This reverts commit 93d20cc.

delucis · 2018-05-12T17:02:39Z

@erquhart Thanks! Removed that block.

Rewrote section, comments addressed.

erquhart · 2018-05-14T15:46:22Z

Thanks again @delucis!

delucis added 11 commits April 29, 2018 02:31

Add mergeMethod option to API config.

324b7fc

Use snake case for merge_method key

a25056b

Add merge_method to mergePR() request

0a39422

Get merge_method setting from backend config

12f409d

Document merge_method backend option

7dcb7e9

Update contributor details.

0598c72

Switch to boolean squash or standard merge config.

db27590

Add config validation for backend.squash_merges option.

93d20cc

Pass squash_merges option to new API() calls.

94ed858

Improve explanation in docs.

c6ab176

Improve explanation in docs.

94964b8

Move squash_merges documentation to Beta Features page.

e8bc69c

verythorough previously requested changes May 4, 2018

View reviewed changes

Simplify documentation language for clarity.

1e379e9

Benaiah approved these changes May 9, 2018

View reviewed changes

erquhart suggested changes May 12, 2018

View reviewed changes

Remove config validation.

48c1b2f

This reverts commit 93d20cc.

update docs

b644032

erquhart approved these changes May 14, 2018

View reviewed changes

erquhart merged commit 0408702 into decaporg:master May 14, 2018

delucis deleted the api-merge-method-config branch May 14, 2018 16:08

delucis mentioned this pull request May 14, 2018

Enable user-configured commit message templates #1359

Merged

1 task

brianlmacdonald pushed a commit to brianlmacdonald/netlify-cms that referenced this pull request May 23, 2018

Add squash merges option for editorial workflow (decaporg#1330)

8c68ea8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add squash merges option for editorial workflow #1330

Add squash merges option for editorial workflow #1330

delucis commented Apr 29, 2018 •

edited

Loading

verythorough commented Apr 29, 2018 •

edited

Loading

verythorough commented Apr 29, 2018

verythorough commented Apr 29, 2018 •

edited

Loading

Benaiah commented May 2, 2018

delucis commented May 2, 2018

Benaiah commented May 2, 2018

delucis commented May 3, 2018 •

edited

Loading

verythorough left a comment

verythorough May 4, 2018

delucis commented May 5, 2018

delucis commented May 11, 2018

erquhart left a comment

erquhart May 12, 2018

delucis commented May 12, 2018

erquhart commented May 14, 2018

Add squash merges option for editorial workflow #1330

Add squash merges option for editorial workflow #1330

Conversation

delucis commented Apr 29, 2018 • edited Loading

Summary

Why?

Considerations

Test plan

Description for the changelog

A picture of a cute animal (not mandatory but encouraged)

verythorough commented Apr 29, 2018 • edited Loading

verythorough commented Apr 29, 2018

verythorough commented Apr 29, 2018 • edited Loading

Benaiah commented May 2, 2018

delucis commented May 2, 2018

Benaiah commented May 2, 2018

delucis commented May 3, 2018 • edited Loading

verythorough left a comment

Choose a reason for hiding this comment

verythorough May 4, 2018

Choose a reason for hiding this comment

delucis commented May 5, 2018

delucis commented May 11, 2018

erquhart left a comment

Choose a reason for hiding this comment

erquhart May 12, 2018

Choose a reason for hiding this comment

delucis commented May 12, 2018

erquhart commented May 14, 2018

delucis commented Apr 29, 2018 •

edited

Loading

verythorough commented Apr 29, 2018 •

edited

Loading

verythorough commented Apr 29, 2018 •

edited

Loading

delucis commented May 3, 2018 •

edited

Loading