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

refactor: move core22 commands for versioned implementation #4586

Merged
merged 3 commits into from
Feb 20, 2024

Conversation

syu-w
Copy link
Contributor

@syu-w syu-w commented Feb 15, 2024

This is a pure rename with no code changes.

This is a pure rename with no code changes.
@syu-w syu-w requested review from mr-cal, cmatsuoka and lengau February 15, 2024 21:54
Copy link

codecov bot commented Feb 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6ff3e0b) 88.83% compared to head (39af6c9) 88.83%.
Report is 3 commits behind head on feature/craft-application.

Additional details and impacted files
@@                    Coverage Diff                     @@
##           feature/craft-application    #4586   +/-   ##
==========================================================
  Coverage                      88.83%   88.83%           
==========================================================
  Files                            327      329    +2     
  Lines                          22033    22040    +7     
==========================================================
+ Hits                           19573    19580    +7     
  Misses                          2460     2460           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tigarmo
Copy link
Contributor

tigarmo commented Feb 16, 2024

@syu-w two things:

  1. Since we're changing core22/legacy code I think we should temporarily run all spread tests to ensure the non-core24 ones are still working. You can change that here.
  2. Thanks for noticing the wrong default command. I see that this opened up some new failures, but I don't think you should investigate them on this PR because it's out-of-scope. Once this is merged I'll review these new errors, as I put in that bug in the first place.

Thanks!

@syu-w
Copy link
Contributor Author

syu-w commented Feb 16, 2024

@syu-w two things:

  1. Since we're changing core22/legacy code I think we should temporarily run all spread tests to ensure the non-core24 ones are still working. You can change that here.
  2. Thanks for noticing the wrong default command. I see that this opened up some new failures, but I don't think you should investigate them on this PR because it's out-of-scope. Once this is merged I'll review these new errors, as I put in that bug in the first place.

Thanks!

OK, full spread tests enabled. I think the new errors are come from changes in the craft-application, as we are using git version as dep.

@syu-w syu-w added the rebase label Feb 16, 2024
@syu-w syu-w requested a review from tigarmo February 16, 2024 19:07
@syu-w
Copy link
Contributor Author

syu-w commented Feb 16, 2024

All errors seems related to craft-application. The LXD related errors are failed to some snapcraft command in LXD.

Copy link
Contributor

@tigarmo tigarmo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@sergiusens sergiusens enabled auto-merge (rebase) February 20, 2024 16:48
@sergiusens sergiusens disabled auto-merge February 20, 2024 16:48
@sergiusens sergiusens merged commit 505e03b into feature/craft-application Feb 20, 2024
7 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants