-
Notifications
You must be signed in to change notification settings - Fork 46
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
Merge craft-application update into main #527
Conversation
Is this the one that updates craft-application to 2.4.0? |
yes |
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.
LGTM. Also moving build-base settings from the extension to user if the extension is set and base is bare sounds like a sensible decision in the long term.
The BuildPlanner has the project's base, build-base and platform declarations, and does all the validation related to those fields.
This cleans-up the code related to functionality that is now implemented upstream. This includes: - registering application plugins; - package-repositories installation (including overlays). It also updates some tests to latest features, like "parallel_build_count".
The brunt of the update is updating usages of platform/build-for to the build plan. This largely affects the image and package services, but the semantics should be unchanged because it's the same data just delivered differently.
The problem is this: the flask extension currently adds a 'build-base' if the 'base' is 'bare' but the 'build-base' is undeclared, and a 'platforms' entry if it's missing, but this behavior breaks with the new version of craft-application because the BuildPlanner processes the yaml _before_ the extensions are applied, and the planner cannot work without a build-base or platform description. The extension behavior of adding a 'build-base' and a 'platforms' item needs to be reviewed: we probably don't want to support this. In the meantime, this commit fixes only the projects that are executed in spread.
17beef2
to
3c0c48d
Compare
Rebased on main, should be ready to rebase-merge when CI completes |
The update to the latest craft-application update was done over time in this
feature/craft-application
branch, which is now ready to be integrated back into main. Only the latest commit needs to be reviewed because we might as well update to the very new2.4.0
- all other commits have already been reviewed in various past PRs.