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

feat(release notes): Show release notes from monorepo #12543

Merged

Conversation

sergVedmak
Copy link
Contributor

@sergVedmak sergVedmak commented Nov 7, 2021

Changes:

  • fix few cache key

  • UPD 11.11.2021 change summary of release notes collapsible section from repoName to depName

  • UPD 11.11.2021 change summary of release notes collapsible section from repoName to repoName (depName), only if pull request updates several dependencies and some dependencies have the same repoName

  • UPD 11.11.2021 change logic to deduplicate release notes

current logic: Iterate over each dependency update in pull request and compare repo source url. If there is another update with the same repo source url, hide the release notes

new logic: Each dependency update has releases with releaseNotes. When the release notes are in the process of being received, I save source of release notes in notesSourceUrl field. It can be link to changelog.md file or gihub/gitlab release api, without specifying a specific version. Then, iterate over each dependency update, get notesSourceUrl from first release notes and check that there is no another dependency with same notesSourceUrl.

Examples:

Update react, react-is, renovate in one pull request. react and react-is sources are located in monorepo, have different sourceDirectory in package.json and have same changelog source. Deduplicate release notes

Снимок экрана 2021-11-11 в 17 34 46

Update our private packages. They are located in monorepo, have different sourceDirectory in package.json and have different changelog.md. Do not deduplicate release notes and add depName to summary of collapsible section

Снимок экрана 2021-11-11 в 17 34 58

Context:

First problem: If there are two packages and they are located in the monorepo, only the first package will receive the update notes. Changes in cache keys fix this problem.

Second problem: Release notes are shown via collapsible section. Summary of this section is repository name. If we group two packages from monorepo in one merge request, will appear two collapsible sections with same summary.

Documentation

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

@sergVedmak
Copy link
Contributor Author

related issue #10954

@rarkins
Copy link
Collaborator

rarkins commented Nov 8, 2021

@sergVedmak thanks for digging into this problem! Can you answer me one question:

If there are two packages from the same monorepo, and with different sourceDirectory, but the same (shared) release notes from the repo, will those duplicated release notes be both displayed, or will they be de-duplicated somehow?

@sergVedmak
Copy link
Contributor Author

@sergVedmak thanks for digging into this problem! Can you answer me one question:

If there are two packages from the same monorepo, and with different sourceDirectory, but the same (shared) release notes from the repo, will those duplicated release notes be both displayed, or will they be de-duplicated somehow?

Yes, both duplicated release notes will be displayed. I will try to find a new solution for this case and update the pull request

@sergVedmak sergVedmak marked this pull request as draft November 9, 2021 15:53
@sergVedmak sergVedmak marked this pull request as ready for review November 11, 2021 14:44
@sergVedmak
Copy link
Contributor Author

sergVedmak commented Nov 11, 2021

I ended up with a new solution and updated the request description. There are a description of the work of the new deduplication logic and screenshots with examples

@sergVedmak
Copy link
Contributor Author

I ended up with a new solution and updated the request description. There are a description of the work of the new deduplication logic and screenshots with examples

@rarkins can you check my changes? (I can't re-request review)

@rarkins
Copy link
Collaborator

rarkins commented Nov 15, 2021

@sergVedmak I will review the code soon, thanks. Can you also describe what changes you found necessary or not necessary to make with changelog caching as a result of these changes?

@sergVedmak
Copy link
Contributor Author

sergVedmak commented Nov 15, 2021

I'm not sure if I understood the question correctly. I don't think that my changes in cache keys must be in changelog(release notes)

One thing: new logic for de-duplicate with notesSourceUrl won't work correctly if there are cached release notes(without notesSourceUrl information) for some package in cache. I tested working with a clean cache

@rarkins
Copy link
Collaborator

rarkins commented Nov 15, 2021

So, we still need to check:

  • Are any changes to changelog caching necessary due to these changes?
  • Will it play nicely with a non-clean cache?

@sergVedmak
Copy link
Contributor Author

  • no
  • no, it don't play nicely always. duplicated release notes may appear in some situations with non-clean cache

@rarkins
Copy link
Collaborator

rarkins commented Nov 15, 2021

In that case we should bump the cache key version so that existing cached notes get expired and new ones fetched and set with the new key

@sergVedmak
Copy link
Contributor Author

can you help me to find the place, where I need to bump the cache key version?

@rarkins
Copy link
Collaborator

rarkins commented Nov 15, 2021

I added an example but currently on mobile so may have missed others which need to be changed too

sergVedmak and others added 2 commits November 15, 2021 17:48
@sergVedmak
Copy link
Contributor Author

Thanks, I have updated one more key

@sergVedmak
Copy link
Contributor Author

is there any news about this?

lib/datasource/types.ts Outdated Show resolved Hide resolved
lib/datasource/types.ts Outdated Show resolved Hide resolved
lib/workers/pr/changelog/types.ts Show resolved Hide resolved
@sergVedmak
Copy link
Contributor Author

@viceice can you check my answers to your comments, please?

Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

see comments

lib/workers/pr/index.ts Outdated Show resolved Hide resolved
lib/workers/types.ts Outdated Show resolved Hide resolved
lib/workers/types.ts Outdated Show resolved Hide resolved
@rarkins rarkins requested a review from viceice November 30, 2021 11:21
Copy link
Collaborator

@rarkins rarkins left a comment

Choose a reason for hiding this comment

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

Looks ok to me, but has this final version been tested e2e in real repos?

Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

Yeah, LGTM. Please do some new e2e tests.

@rarkins
Copy link
Collaborator

rarkins commented Dec 1, 2021

@sergVedmak please let us know once you're confident to merge, thanks

@sergVedmak
Copy link
Contributor Author

I created two repo with same dependencies

sergVedmak/emotion-renovate#1 - PR from Renovate Github App (current behavior). Release notes shown only for first dependency

sergVedmak/emotion-renovate-2#2 - PR from local running renovate with final version of this changes . Each dependency has its own release notes

@sergVedmak
Copy link
Contributor Author

Is the test from the previous comment enough? or do I need to check something else?

@rarkins rarkins changed the title fix(release notes): Show release notes from monorepo feat(release notes): Show release notes from monorepo Dec 2, 2021
@rarkins rarkins enabled auto-merge (squash) December 2, 2021 11:22
@rarkins rarkins merged commit e8e99ef into renovatebot:main Dec 2, 2021
@rarkins
Copy link
Collaborator

rarkins commented Dec 2, 2021

Thanks @sergVedmak for a really great PR

@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 29.30.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants