-
Notifications
You must be signed in to change notification settings - Fork 59
Check branch version #675
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
base: dev
Are you sure you want to change the base?
Check branch version #675
Conversation
…o avoid compilation errors not finding the enumeration when editing the class. * Convert all public fields into properties * Encapsulate all module level variables into a internal state UDT. This does make the code more lengthy but does make it easy to discover where properties are used.
… the dictionary items for clarity, improve the encapsulation of the index's internal state, introduce checks for mergability and branch consistency. Update the main form to check the IsMergable instead of FullBuildDate for comprehensive check.
…integration into CheckBranchVersion
…using the testing database to fail. (likely regressed from joyfullservice#667)
…riginal version of using cd embedded in the script command will not be completely effective especially if the project is on some other drive because in command prompt, one must change the drive and then change the directory. That causes the git command to then fail on those projects housed in other drives.
…nd to communicate specific reason why merge cannot proceed, differentiating between missing a full build, being on an incompatible branch, or asking the user to confirm if they intend to export from a different branch.
|
oh this is cool. I like it! I also appreciate that this creates a mechanism to do other checks for merge/build capable, which don't require a ton of retooling externally to the function |
| ' not work correctly due to objects that depend upon | ||
| ' each other.) | ||
| If VCSIndex.FullBuildDate = 0 Then | ||
| If VCSIndex.IsMergable = False Then |
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.
Would it be cleaner to just say If Not VCSIndex.IsMergable Then or do you prefer the explicit statement of t/f here?
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.
My habit has been to use something = False especially when the something is a non Boolean variable in which case the Not operator won't do the right thing. Not applicable since it's a Boolean function but consistency.
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.
Aha, makes sense!
Closes #653
This enables the check for the branch.
Generally:
The index will now record the branch name & hash at the time of build.
When merge is requested, the index is compared against the repo's branch data.
If the branch is different, we then check the hash. If it's the same, it's compatible branch and in that case, we request a confirmation from the user to confirm they meant to build from that branch.
In other cases, we reject the merge and provide a detailed message why they cannot.
I'm posting this for reviews by others.
Note: This includes a minor bug fixes for document properties (missing properties which was causing the testing database to fail) as well as a fix for the git integration which was not working on a git repository that wasn't on a C drive. The git command had a
cd <repository root>which isn't effective because in the command prompt, one must do acd <drive>first followed bycd <path>. To avoid that ugliness, we instead set theCurrentDirectoryof theWshShell.