Skip to content
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.

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

ci(changelog): add deprecation commits to changelog automation #10346

Merged

Conversation

DitwanP
Copy link
Contributor

@DitwanP DitwanP commented Sep 19, 2024

Related Issue: #9975

Summary

Adds custom type list to lerna and release-please config files, and add a deprecation type to the list. This should allow for tracking deprecations in the change-logs

@github-actions github-actions bot added the chore Issues with changes that don't modify src or test files. label Sep 19, 2024
@DitwanP DitwanP added the skip visual snapshots Pull requests that do not need visual regression testing. label Sep 19, 2024
@DitwanP DitwanP marked this pull request as ready for review September 19, 2024 18:37
lerna.json Outdated Show resolved Hide resolved
lerna.json Outdated Show resolved Hide resolved
…nto dit13711/9975-add-deprection-commits-to-changelog-automation
Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue seems primarily focused on documentation, so introducing a brand-new type for deprecations could be confusing (e.g., we sometimes deprecate purely with doc, and other times with a feature or fix).

The type object supports specifying a scope. Could we test if the following would address the request?

 {"type": "docs", "hidden": true},
 {"type": "docs", "scope": "deprecation", "hidden": false},

@benelan
Copy link
Member

benelan commented Sep 23, 2024

The issue seems primarily focused on documentation, so introducing a brand-new type for deprecations could be confusing (e.g., we sometimes deprecate purely with doc, and other times with a feature or fix).

The type object supports specifying a scope. Could we test if the following would address the request?

Yeah I like this idea, more specifically can you test if this works:

 { "type": "docs", "scope": "deprecation", "section": "Deprecations" },

@jcfranco
Copy link
Member

Didn't realize I left out my suggestion 😅 (updated now), but let's proceed w/ @benelan's.

@DitwanP
Copy link
Contributor Author

DitwanP commented Sep 23, 2024

@jcfranco, @benelan

Yeah I like this idea, more specifically can you test if this works:
{ "type": "docs", "scope": "deprecation", "section": "Deprecations" },

So I want to make sure I understand what this suggestion and Franco's are asking for. Would this be a replacement for the deprecation type, so we would instead use the "docs" type with a scope of deprecation?

If so, it should be noted that it the change-log would only pickup deprecation commits with a format like this "docs(deprecation): This is a deprecation...." and would show up under the deprecations section like this:
image

Note that the scope would then always be "deprecation" instead of showing more specific scopes like the other change-log types (feat, fix).
image

@jcfranco
Copy link
Member

Solid point. We would definitely lose some flexibility with the docs(deprecation) approach.

@DitwanP @benelan WDYT about tweaking the original solution to be more explicit, like deprecation-doc or similar, to indicate it's doc-only?

@DitwanP
Copy link
Contributor Author

DitwanP commented Sep 24, 2024

Solid point. We would definitely lose some flexibility with the docs(deprecation) approach.

@DitwanP @benelan WDYT about tweaking the original solution to be more explicit, like deprecation-doc or similar, to indicate it's doc-only?

@jcfranco
Do you mean have one type that's for deprecate and another for deprecate-doc?

You mentioned that sometimes we deprecate with purely docs and other times with a feat or fix, I thought that in those feat/fix cases is where the deprecate would come into play so that on the changelog there's an explicit section to list what was deprecated, but maybe that's not the case?

@jcfranco
Copy link
Member

Do you mean have one type that's for deprecate and another for deprecate-doc?

I was suggesting the latter.

I thought that in those feat/fix cases is where the deprecate would come into play so that on the changelog there's an explicit section to list what was deprecated, but maybe that's not the case?

We can only use one type per commit, so I don't think we can easily split up deprecation code changes from a feat/fix.
For example, how would we use deprecate with this feat PR? Maybe I'm missing something? 🤔

@DitwanP
Copy link
Contributor Author

DitwanP commented Sep 24, 2024

For example, how would we use deprecate with this feat PR? Maybe I'm missing something? 🤔

So I thought something like that would now be

deprecate: remove widthScale....

or

deprecate(widthScale, heightScale): something something..."

@jcfranco
Copy link
Member

That would work if the change is purely to deprecate a component member, but we often handle deprecations within a fix or feat. In the example above, the changelog would no longer display a feat entry.

@DitwanP
Copy link
Contributor Author

DitwanP commented Sep 25, 2024

That would work if the change is purely to deprecate a component member, but we often handle deprecations within a fix or feat. In the example above, the changelog would no longer display a feat entry.

@jcfranco

🤔 I get that we currently sometimes handle deprecations like that but I guess I'm confused because I thought we would now just do the deprecation separate from the feature or a fix PRs.

What do you have in mind for how having deprecate-docs would change things? Would it be used for the purely deprecation PRs and we would still sometimes bundle deprecations with feat/fix? Because in those cases we would just end up with the changelog listing deprecations under feat/fix sections instead of deprecations.

@jcfranco
Copy link
Member

I thought we would now just do the deprecation separate from the feature or a fix PRs.

We can, but this adds, IMO, unnecessary overhead. In the example above, we need to add code to handle both the new prop and the deprecated one in the same PR. Not sure how we could separate those cleanly. Also, it'd be odd to not add a deprecation note when the new props are added.

…nto dit13711/9975-add-deprection-commits-to-changelog-automation
…nto dit13711/9975-add-deprection-commits-to-changelog-automation
@DitwanP
Copy link
Contributor Author

DitwanP commented Sep 25, 2024

@benelan the conversation between Franco and I continued on teams and we wanted your input on the pending solution. We currently are planning to use deprecate-docs in the manner described in this pending wiki update:
image

Do you have any objections to the empty commit option or perhaps a suggestion for another route we could take.

@benelan
Copy link
Member

benelan commented Sep 26, 2024

Yeah that works for me. We can add multiple changelog entries from a single PR in release-please:
https://github.com/googleapis/release-please#what-if-my-pr-contains-multiple-fixes-or-features

So we could create the PR title as the type doc/fix/feat, but include a deprecate tag as well.

@DitwanP
Copy link
Contributor Author

DitwanP commented Sep 26, 2024

@jcfranco I've made the changes based on Ben's suggestions.

Also updated the wording on the pending wiki update to reflect the proposed process, LMK what you think:

When a pull request is strictly intended to deprecate an existing feature or component, use the deprecate commit type. This type is reserved for noting changes under the "Deprecations" section of the changelog.

If deprecations are part of a broader change, such as introducing a new feature or making a fix, a deprecate entry can be added to the body of the PR following the format described in the release-please documentation.

@jcfranco
Copy link
Member

That looks great! Thanks to both of youse!

The only note I have is to consider adding some example snippets to the wiki update.

Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝
📝⛔⛔⛔⛔📝⛔📝📝📝📝📝⛔📝⛔⛔⛔⛔📝📝⛔⛔⛔📝📝⛔⛔📝📝⛔📝📝📝⛔📝⛔⛔⛔⛔📝⛔📝
📝⛔📝📝⛔📝⛔📝📝📝📝📝⛔📝⛔📝📝📝📝⛔📝📝📝📝⛔📝📝⛔📝⛔⛔📝⛔⛔📝⛔📝📝📝📝⛔📝
📝⛔⛔⛔⛔📝⛔📝📝⛔📝📝⛔📝⛔⛔⛔📝📝📝⛔⛔📝📝⛔📝📝⛔📝⛔📝⛔📝⛔📝⛔⛔⛔📝📝⛔📝
📝⛔📝📝⛔📝⛔📝⛔📝⛔📝⛔📝⛔📝📝📝📝📝📝📝⛔📝⛔📝📝⛔📝⛔📝📝📝⛔📝⛔📝📝📝📝📝📝
📝⛔📝📝⛔📝📝⛔📝📝📝⛔📝📝⛔⛔⛔⛔📝⛔⛔⛔📝📝📝⛔⛔📝📝⛔📝📝📝⛔📝⛔⛔⛔⛔📝⛔📝
📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝📝

@DitwanP DitwanP merged commit 89194b0 into dev Sep 26, 2024
11 checks passed
@DitwanP DitwanP deleted the dit13711/9975-add-deprection-commits-to-changelog-automation branch September 26, 2024 21:21
benelan added a commit that referenced this pull request Sep 26, 2024
…lid-border-color

* origin/dev:
  ci(changelog): add deprecation commits to changelog automation (#10346)
  fix: properly set aria attributes on components (#10404)
  feat: add parcel parameter (#10384)
jcfranco added a commit that referenced this pull request Sep 27, 2024
)

**Related Issue:** N/A

## Summary

Updates our semantic PR check to allow the new commit type added in
#10346.
benelan added a commit that referenced this pull request Sep 30, 2024
…estone-estimates

* origin/dev: (26 commits)
  test: fix `window.getComputedStyle` arguments (#10424)
  test(card): remove redundant spy and setup wrapper element (#10429)
  test(heading): avoid `newSpecPage` usage to ease Lumina migration (#10431)
  refactor(themed): drop broken, unused, regexp arg support (#10425)
  docs: component token description consistency (#10430)
  refactor: prevent mixed Sass mixed declaration warnings (#10426)
  deprecate: deprecate `enforce-ref-last-prop` rule (#10421)
  chore(actions): add custom `deprecate` type to semantic-pr check (#10427)
  chore(issue templates): add arcgis data pipelines to team dropdown (#10418)
  fix(tooltip): closed tooltips should not reappear (#10420)
  feat: add dashboard-graph (#10417)
  fix(input-time-zone): fix region mode quirks after update (#10413)
  fix(text-area): ensure border-color token doesn't override invalid styles (#10390)
  ci(changelog): add deprecation commits to changelog automation (#10346)
  fix: properly set aria attributes on components (#10404)
  feat: add parcel parameter (#10384)
  feat(alert): apply --calcite-alert-corner-radius to internal close button (#10388)
  feat(dialog, panel): Add css properties for background-color (#10387)
  fix: remove aria-disabled from components where necessary (#10374)
  feat(action-group, block, panel): add `menuPlacement` and `menuFlipPlacements` properties (#10249)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Issues with changes that don't modify src or test files. skip visual snapshots Pull requests that do not need visual regression testing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants