beware of squashing single-commit PRs #17

zeke · 2018-07-17T16:24:07Z

Also, an edge case for squash commits (via @francisfuzz):

I was reminded of something I learned about how we treat squash merges recently and would like to share it here in the case it hasn’t been considered by the Electron team.

Squashing a pull request with one commit would use that commit’s message, but squashing a pull request with more than one commit will use the pull request’s title for the commit message.

frabonomi · 2019-01-24T13:23:44Z

Any update? We could take a similar approach as this.

zeke · 2019-01-24T17:01:02Z

I haven't made time to work on this, but if anyone wants to open a PR for it, I'd be happy to review, merge, and deploy it!

frabonomi · 2019-02-02T16:30:19Z

Ok, I'm going to work on it and send a PR!

zeke · 2019-02-02T16:37:07Z

Sounds good!

frabonomi · 2019-02-02T18:33:24Z

Sorry @zeke, just to better understand the purpose of this repo: in which case would one use the default options and allow merging whenever at least one commit has a semantic message or the title of the PR is semantic?

For this fix what would you suggest to do:

fix the behavior of the handler when titleOnly option set to true in order to check by default the PR title, but in case there's only one commit then check the commit's message
create a squashMerging option to define the separate behavior

I feel the first option would be the best, but then the titleOnly name doesn't make much sense anymore. And I'm also not sure what was the rationale behind that option.

kewah · 2019-07-09T12:17:32Z

@zeke it seems to have been fixed on GitHub side. I ran a test and it uses the PR's title for the commit message 👍

EDIT: It worked because I use Refined GitHub as @christopherscott pointed out #17 (comment)

PR:

Commit:

piratefsh · 2019-08-07T20:59:41Z

hi @kewah was your merge made as a squash and merge? I'm still facing this issue where GH still autopopulates single-commit squashes with the commit message instead of the PR title (see screenshot)

kewah · 2019-08-16T07:50:57Z

@piratefsh yes, it was a squashed and merged. I only have that enabled

christopherscott · 2019-08-19T15:28:21Z

@piratefsh I also don't see that this has changed - single commits, when squashed and merged, still seem to default to the commit title

@kewah are you using Refined Github by any chance?

kewah · 2019-08-19T15:29:34Z

@christopherscott oh right! my bad, I do use Refined GitHub so that explains it

zeke · 2019-08-19T23:25:54Z

Yep this is still an issue. Just bit me again. Would love a PR for this if anyone has the time!

zeke · 2019-08-25T02:05:29Z

Hey y'all. I just deployed a fix for this with #58. Give it a try and let me know if you have any issues!

zeke · 2020-07-07T22:23:44Z

#58 had to be rolled back, so this is still an issue.

zeke · 2020-07-08T21:07:58Z

It works!

kunalnagar · 2020-10-02T14:28:36Z

@zeke

Hi there,

I'm having issues with single commit PRs still. I have no config file and am using the default GitHub app.

kunalnagar · 2020-10-15T00:25:30Z

@zeke just checking in on what I posted above ☝️

…nents (#5230) **Related Issue:** #4442 ## Summary Adds consistency to our API documentation within some of the t-named components, including: - tab - tab-nav - tab-title - tabs - tile - tile-select - tile-select-group - time-picker - tip - tip-group - tip-manager

… of the time for rtl locales (#5246) **Related Issue:** #4957 ## Summary Makes sure the meridiem location is determined on load.

**Related Issue:** #5209 ## Summary Sorry I hit merge before I saw this comment https://github.com/Esri/calcite-components/pull/5209/files#r953105006

**Related Issue:** NA ## Summary Unfortunately we need to temporarily disable screener to prevent insane overage fees. We are working on a short term solution to turn screener back on, and a mid-term solution to switch to Chromatic. Screener can still be run on a branch manually following these steps: https://docs.github.com/en/actions/managing-workflow-runs/manually-running-a-workflow For now, please try not to manually run screener unless absolutely necessary. We only have around 35 runs per month at our current snapshot count. We will be working to reduce the number of stories so we can reenable screener PR checks in the short term.

**Related Issue:** # ## Summary We have a GitHub Action that closes a milestone when all of the issues are completed. Currently it is only triggered when an issue is closed. Sometimes an issue does not pass the QA verification and the issue needs to be moved to the current sprint or backburner. With this PR the milestone will properly close.

…5228) **Related Issue:** #3818 ## Summary - Added additional information to the contributing and documentation convention. - Fixed outdated info

**Related Issue:** # ## Summary How did we ever live w/o optional chaining :'( Error: https://github.com/Esri/calcite-components/runs/8052344919?check_suite_focus=true#step:2:44

**Related Issue:** #5253 ## Summary This PR cleanups up and combines stories to lower our snapshot count below 40k per month. - Append `_NoTest` to story names that we don't want screener to snapshot. - Append `_TestOnly` to story names that we do not want to appear in the [Storybook deployment on github pages](https://esri.github.io/calcite-components/?path=/story/overview-home--page). - Set up filters to accomplish the above two points. The filter determines which stories to skip using a boolean environment variable: `STORYBOOK_SCREENSHOT_TEST_BUILD`. The default is `NoTest`, and the env var is set to `true` in the [`pr-screener` workflow](https://github.com/Esri/calcite-components/blob/master/.github/workflows/pr-screener.yml). - Combine stories, e.g. if a component has "DarkTheme" and "RTL" snapshots, we combined them into a single `darkThemeRTL_TestOnly` snapshot. - Consistency cleanups, including camel case story names and using "simple" as the name for the first story (rather than "basic").  Co-authored-by: JC Franco <jfranco@esri.com> Co-authored-by: Erik Harper <eharperdicianno@esri.com>

**Related Issue:** #5257 ## Summary Some additional cleanup. I nuked a lot of the combinations but we can add some back once we are out of crisis mode

**Related Issue:** #5253 ## Summary This PR refines the PR linked above. Here are the conditions for when Screener runs on a PR: - Screener will run when when the `pr ready for visual snapshots` label is added to a normal PR (not draft PRs) - Screener will run when a draft PR with the `pr ready for visual snapshots` label is marked as "Ready to review" When someone pushes changes to a PR that has the `pr ready for visual snapshots label` (meaning snapshots already ran), __Screener will NOT run but it WILL show a green checkmark__ allowing the author to merge the PR. This is so that PRs can be merged w/o another Screener run if there are some small cleanups needed. If you need to rerun screener, remove and re-add the `pr ready for visual snapshots` label. Screener will still run on every push to `master`. This should provide a back up in case a visual regression slips through due to the point above. I will monitor the Screener runs on `master` while these limitations are in place.

**Related Issue:** ## Summary Allow screener to run manually w/o skipping due to strict conditionals

**Related Issue:** # ## Summary Designers are running into issues getting an environment set up to run the demos locally, so I'm clarifying a few things in the contributing guide.

…inputs (#5758) **Related Issue:** #5207 ## Summary This PR fixes an issue where the input boxes are completely empty when the `valueAsDate` property is directly changed with javascript when using the `range` prop.  Co-authored-by: Erik Harper <eriklharper@pm.me>

**Related Issue:** # ## Summary I forgot to pass the diff threshold GH secret to the chromatic workflow's environment variables 😅 That explains why bumping up the value wasn't doing anything

…ccept, multiple, pattern (#5807) **Related Issue:** #4079 ## Summary Adds native input attributes: - [multiple](https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/multiple) to `calcite-input` for the types specified on MDN - [accept](https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/accept) to `calcite-input` for the types specified on MDN - [pattern](https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/pattern) to `calcite-input-text` and `calcite-input` for the types specified on MDN - [autocomplete](https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/autocomplete) to `calcite-input-number`, `calcite-input-text`, and `calcite-input` for the types specified on MDN ## Note just saw on MDN after committing that Safari may not support `autocomplete`: https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/autocomplete#browser_compatibility Should I remove it from the PR?

…upSeparator (#5828) **Related Issue:** #5648 ## Summary `calcite-pagination` was only formatting (localizing) the page numbers when the groupSeparator property was set to `true`. Now lang/numberingSystem work correctly with or without groupSeparator

…omatic errors (#5839) **Related Issue:** #5833 ## Summary Prevent the whitespace storybook addon from getting bumped since it caused issues with chromatic: https://github.com/Esri/calcite-components/actions/runs/3577241314/jobs/6016009432#step:6:207

**Related Issue:** # ## Summary Some more changes to stop text aliasing from triggering Chromatic diffs

**Related Issue:** # ## Summary There is a timing error, where if you sync a branch and change a label right afterwards, your change will be undone. This is because the `pr-bot` was adding the conventional commit type label to the existing ones, but when it started the run your change diudn't exist yet. ![image](https://user-images.githubusercontent.com/10986395/204685262-02fd953a-8904-40fe-b010-3b8c04e9d993.png) I found an API endpoint that just adds labels, without the potential to accidently remove existing ones.

**Related Issue:** #5812 ## Summary The image service we were using for the card's thumbnail in the custom theme story is/was down, which caused CI failures. For example: https://www.chromatic.com/test?appId=6266d45509d7eb004aa254fb&id=6386b5d2e340d812b9c9a134 I switched it out to use our placeholderImage util.

**Related Issue:** #4442 ## Summary 1. Includes consistency across API references, this PR finalizes the style guide across all components. 2. Also includes the removal of the optional "`?`" JSDoc tags for the components listed below. - A separate PR will be created for the remaining components. ### Components included: - ` date-picker` - `dropdown` - `dropdown-group` - `dropdown-item` - `fab` - `filter` - `flow` - `flow-item` - `icon` - `inline-editable`

**Related Issue:** N/A ## Summary Add contributing guidance for devs on time estimates pertaining to the `estimate-#` label. Adds estimates for: - 1 - 2 - 3 - 5 - 8 - 13 - 21, and - 34

**Related Issue:** # ## Summary

@dasa

**Related Issue:** # ## Summary Creating this PR based on the Teams discussion about increasing our `target`. I confirmed with @dasa that these changes are safe for our stakeholders. But any `target` higher than 2020 has the potential to cause them problems. lib 2021 has some useful stuff we can benefit from, like `Promise.any`, `string.prototype.replaceAll`, and logical assignment operators.

**Related Issue:** # ## Summary I'm disabling some of the milestone automation for now. We recently changed the way we work with milestones, which is causing funky behavior. The actions were created under the assumption that the only milestones with due dates would be for our 2 week sprints. I can change the actions if we are deciding to change how we use milestones, but we may want to wait for Brittney to return before making that decision, since that is her area of expertise.

zeke added the bug Something isn't working label Jul 17, 2018

zeke changed the title ~~disregard PR title if PR contains only one commit~~ beware of squashing single-commit PRs Jul 26, 2018

zeke mentioned this issue Jul 26, 2018

Adapt checks to fit repo's merge strategy #11

Closed

zeke mentioned this issue Nov 3, 2018

The bot is not approving the PR with the correct title #23

Closed

zeke mentioned this issue Feb 28, 2019

Why not both? #42

Closed

christopherscott mentioned this issue Aug 21, 2019

feat: add squashMessage option #56

Closed

zeke mentioned this issue Aug 25, 2019

fix: set pending status on single-commit PRs with a solitary unsemantic commit #58

Merged

zeke closed this as completed in #58 Aug 25, 2019

ghost mentioned this issue Oct 31, 2019

Automated way to follow conventional commits when merging PRs with a single commit vectordotdev/vector#1128

Closed

zeke reopened this Jul 8, 2020

zeke mentioned this issue Jul 8, 2020

fix: disallow squashing of unsemantic single-commit PRs #105

Merged

zeke closed this as completed in #105 Jul 8, 2020

Nadock mentioned this issue Jul 13, 2020

Single commit PRs blocked #106

Closed

Bleggbeard mentioned this issue Aug 3, 2020

fix: Fail on single unsemantic non-merge commit #109

Merged

benelan mentioned this issue Dec 21, 2021

revert: use postcss to ensure logical prop usage Esri/calcite-design-system#3747

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

beware of squashing single-commit PRs #17

beware of squashing single-commit PRs #17

zeke commented Jul 17, 2018

frabonomi commented Jan 24, 2019 •

edited

Loading

zeke commented Jan 24, 2019

frabonomi commented Feb 2, 2019

zeke commented Feb 2, 2019

frabonomi commented Feb 2, 2019

kewah commented Jul 9, 2019 •

edited

Loading

piratefsh commented Aug 7, 2019

kewah commented Aug 16, 2019

christopherscott commented Aug 19, 2019 •

edited

Loading

kewah commented Aug 19, 2019

zeke commented Aug 19, 2019

zeke commented Aug 25, 2019

zeke commented Jul 7, 2020

zeke commented Jul 8, 2020

kunalnagar commented Oct 2, 2020 •

edited

Loading

kunalnagar commented Oct 15, 2020

beware of squashing single-commit PRs #17

beware of squashing single-commit PRs #17

Comments

zeke commented Jul 17, 2018

frabonomi commented Jan 24, 2019 • edited Loading

zeke commented Jan 24, 2019

frabonomi commented Feb 2, 2019

zeke commented Feb 2, 2019

frabonomi commented Feb 2, 2019

kewah commented Jul 9, 2019 • edited Loading

piratefsh commented Aug 7, 2019

kewah commented Aug 16, 2019

christopherscott commented Aug 19, 2019 • edited Loading

kewah commented Aug 19, 2019

zeke commented Aug 19, 2019

zeke commented Aug 25, 2019

zeke commented Jul 7, 2020

zeke commented Jul 8, 2020

kunalnagar commented Oct 2, 2020 • edited Loading

kunalnagar commented Oct 15, 2020

frabonomi commented Jan 24, 2019 •

edited

Loading

kewah commented Jul 9, 2019 •

edited

Loading

christopherscott commented Aug 19, 2019 •

edited

Loading

kunalnagar commented Oct 2, 2020 •

edited

Loading