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.

Sign up for GitHub

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

Jump to bottom

updateChangelog does nothing if isReleaseCandidate is false #180

Closed
mcmire opened this issue Dec 7, 2023 · 3 comments · Fixed by #181
Closed

updateChangelog does nothing if isReleaseCandidate is false #180

mcmire opened this issue Dec 7, 2023 · 3 comments · Fixed by #181
Assignees
@MajorLift
Labels
bug Something isn't working team-wallet-framework

Comments

@mcmire
Copy link
Contributor

mcmire commented Dec 7, 2023 •
edited
Loading

updateChangelog seems to have been changed in #158 such that it does nothing if isReleaseCandidate is false.

Curiously, these changes did not cause the tests to fail. That could indicate some missing test cases. We should add the missing tests and revert the behavior. It would also be helpful to go back and update the changelog entry for this PR and indicate that the change was breaking.
@mcmire mcmire mentioned this issue Dec 7, 2023
Merged
@mcmire mcmire transferred this issue from MetaMask/create-release-branch Dec 7, 2023
@desi
Copy link
Contributor

desi commented Dec 7, 2023

Hey team! Please add your planning poker estimate with Zenhub @cryptodev-2s @Gudahtt @MajorLift @mcmire @mikesposito

@desi desi added the bug Something isn't working label Dec 7, 2023
@MajorLift MajorLift self-assigned this Dec 8, 2023
@MajorLift MajorLift mentioned this issue Dec 8, 2023
Merged
@Gudahtt
Copy link
Member

Gudahtt commented Dec 14, 2023

Could we update this issue to describe the problem, rather than the solution? The problem I think is that the update command always results in zero updates when --rc is omitted.

@Gudahtt Gudahtt mentioned this issue Dec 14, 2023
Merged
@mcmire
Copy link
Contributor Author

mcmire commented Dec 14, 2023

Yes, that's right. I've updated the PR description to match.

@mcmire mcmire changed the title Revert changes to updateChangelog updateChangelog does nothing if isReleaseCandidate is false Jan 29, 2024
MajorLift added a commit that referenced this issue Feb 8, 2024
@MajorLift
Fix bug introduced to updateChangelog in #158 (#181)
5ebfe97 
## Motivation

#158 incorrectly refactored the branches in updateChangelog, resulting in a state where new changelog entries weren't added to the return value if isReleaseCandidate is false.

## Explanation

- This error is fixed by moving the logic for retrieving new entries out of the if (isReleaseCandidate) block.
- The code for retrieving new entries is also extracted into its own method: getNewChangeEntries, abstracting away details that obscured the flow of the method's core logic.
- To verify that the buggy logic is correctly restored, it's necessary to compare the current state of updateChangelog to its state before the bug was introduced. For this, see: [diff link](https://github.com/MetaMask/auto-changelog/compare/e8df1ec717f534c8fe84c46ea86a847fa5a32973..da39a58a55571cf4e8a03e28096aa12c02c75f77#diff-4228e8302e41dd1c51e813af8368efdcb40b3d9db3e3f21f417ae87275d9f389R249-R316).

## References

- Closes #180
- Followed by #188
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@MajorLift MajorLift

Labels
bug Something isn't working team-wallet-framework
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix bug introduced to updateChangelog in #158 MetaMask/auto-changelog
4 participants
@mcmire @desi @Gudahtt @MajorLift