- Notifications
You must be signed in to change notification settings - Fork1.8k
feat: implement tags push inverifyAuth dry-run operation and isolate branch status check#3447
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Conversation
smallish suggestion: "refactor" is a word that is often confused in the software community. used correctly, it should mean a change to the structure of the code that does not change the behavior of the code. in this case, the behavior is changed, so we should be careful not to use "refactor" in this case. i would consider this a feature instead |
Very true... well noted! Thank you 😁 |
verifyAuth dry-run operation and isolate branch status check| if(!(awaitisBranchUpToDate(options.repositoryUrl,context.branch.name,{ cwd, env}))){ | ||
| logger.log( | ||
| `The local branch${context.branch.name} is behind the remote one, therefore a new version won't be published.` | ||
| ); | ||
| returnfalse; | ||
| } |
babblebeyJul 12, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Hi hi@travi,
This check used to be conditionally executed i.e. it only runs when thepush permission check in theverifyAuth logic fails or throws an error... This PR isolates the check bringing it into the verifications during the release process regardless of the push permission check result... What do you think??
- Is this alright??
- Can it be consider a breaking change?? If so - can we align with the
beta?? - What you think about having
divergedrepo/branches considered as not up-to-date in the context of publishing releases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
thanks for bringing my attention back to this one
- Can it be consider a breaking change?? If so - can we align with the
beta??
help me understand what would make this breaking. if the push permission check succeeds, the flow would continue to this point. if it fails, that check throws an error before reaching this point, correct? it seems like the actual behavior remains essentially the same regarding this detail, so that seems non-breaking to me. is this more than just a fix? if so, help me see what i'm overlooking
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
If you put that way, It really does behave similarly. But slightly difference is that if the push permission passes... We don't really care much whether the branch is up to date or not.
This change here insists on checking if the branch is up to date after the push permission passes.
Now, is that change break worthy??? 🤔
Uh oh!
There was an error while loading.Please reload this page.
This PR refactors the
verifyAuthfunction replacing the normal push dry-run (which could result in a branch outdated error/notice) with a tags dry-run push. It does this by separating the push permission check operation in theverifyAuthlogic from theisBranchUpdatedlogic that lived in thesame execution block via atry-catchinto standalone operations; this branch status check only occured when the push permission check fails.Changes Made
verifyAuthlogic to do a tags dry run instead of a branch dry-run pushverifyAuthto a standalone block to throw a"EGITNOPERMISSION"if no push permission is foundisBranchUpdatedhelper to ensure it checks for cases where local branch is either behind, ahead or diverged of head remote branch and return appropriate boolean response which in this case is...false- when local branch is behind remote branchtrue- when local branch is ahead of remote branchfalse- when local branch and remote branch have commits that the other doesn'tfalsewhen branch is considered not up-to-date - with a messageBREAKING CHANGE
semantic-releasenow checks if your local branch is up-to-date with your remote repo branch regardless of your push permission verification result.Related Issue
Fixes#3437