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

(freecad) Updated to remove files and dev version issues #1948

Closed
wants to merge 7 commits into from

Conversation

RedBaron2
Copy link
Contributor

@RedBaron2 RedBaron2 commented Jul 14, 2022

@gep13 @AdmiringWorm @luzpaz @adrianinsaval

Description

This will fix the package from trying to push as a embedded package as it is not an embedded package. It happened because I forgot to update for removing the zip files. This will also allow for direct updating of the dev minor version as a separate variable from the update script. This will only be needed if the update helper logic fails.

Motivation and Context

This will fix issue #1941

How Has this Been Tested?

Intranet and appveyor

Screenshot (if appropriate, usually isn't needed):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Migrated package (a package has been migrated from another repository)

Checklist:

  • My code follows the code style of this repository.
  • My change requires a change to documentation (this usually means the notes in the description of a package).
  • I have updated the documentation accordingly (this usually means the notes in the description of a package).
  • I have updated the package description and it is less than 4000 characters.
  • All files are up to date with the latest Contributing Guidelines
  • The added/modified package passed install/uninstall in the chocolatey test environment.
  • The changes only affect a single package (not including meta package).

Original Location

Added `freecadMinor` for updating the minor version number should it not be processed with logic
@AppVeyorBot
Copy link

✅ Package verification completed without issues. PR is now pending human review

automatic/freecad/update_helper.ps1 Outdated Show resolved Hide resolved
automatic/freecad/update_helper.ps1 Outdated Show resolved Hide resolved
@RedBaron2
Copy link
Contributor Author

@AdmiringWorm
Closing this PR due to the concerns raised by @adrianinsaval about whatif in fringe cases.

@RedBaron2 RedBaron2 closed this Jul 27, 2022
@fkrauthan
Copy link
Contributor

@RedBaron2 Are you gonna open a new Merge request or will freecad stay broken?

@RedBaron2
Copy link
Contributor Author

I'll be opening a new PR soon

@adrianinsaval
Copy link
Contributor

IMO while we work on the dev releases at least the portion removing the zip files should be merged so we can get the stable update in

@RedBaron2
Copy link
Contributor Author

I'm re-opening this PR after help from @adrianinsaval in approving versioning issues.

@RedBaron2 RedBaron2 reopened this Jul 28, 2022
@AppVeyorBot
Copy link

✅ Package verification completed without issues. PR is now pending human review

1 similar comment
@AppVeyorBot
Copy link

✅ Package verification completed without issues. PR is now pending human review

@RedBaron2
Copy link
Contributor Author

@AdmiringWorm
This seems to be ready for your review. Thanks for taking the time.

automatic/freecad/update_helper.ps1 Outdated Show resolved Hide resolved
@AdmiringWorm
Copy link
Member

Also, it would be great if you could add yourself to the CODEOWNERS file located in the .github directory. At some point in the future, it will be required that at least one maintainer is listed there for a pull request to be pulled into the repository, and since you have done the majority of the work for this package, you look to be the appropriate candidate for this.

Do note that this file will be used going forward for who to request pull requests from.
This is not yet a requirement for anyone to be added, though, so you do not have to add anything if you do not want to be listed there.

@AppVeyorBot
Copy link

❌ Package verification failed, please review the Appveyor Logs and the provided Artifacts before requesting a human reviewer to take a look.

@AppVeyorBot
Copy link

❌ Package verification failed, please review the Appveyor Logs and the provided Artifacts before requesting a human reviewer to take a look.

@AppVeyorBot
Copy link

❌ Package verification failed, please review the Appveyor Logs and the provided Artifacts before requesting a human reviewer to take a look.

@RedBaron2
Copy link
Contributor Author

I don't know what is going on to cause appveyor to crash the AU updating with error 0

I'm going to Start a new PR and see if appveyor likes those changes

@RedBaron2
Copy link
Contributor Author

@AdmiringWorm
I can reopen this but since take 2 is open I'll close this PR

@RedBaron2 RedBaron2 closed this Aug 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants