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

feat[close #24] Add --dry-run option #173

Merged
merged 3 commits into from
Dec 18, 2023
Merged

Conversation

mirkobrombin
Copy link
Member

@mirkobrombin mirkobrombin commented Dec 18, 2023

close #24

I am unable to test due a problem with ABRoot which afflict my installation since weeks:

  ERROR   building at STEP "LABEL maintainer 'Generated by ABRoot'": error decoding response from copier subprocess: invalid character '(' looking for beginning of value
Error: building at STEP "LABEL maintainer 'Generated by ABRoot'": error decoding response from copier subprocess: invalid character '(' looking for beginning of value

@mirkobrombin mirkobrombin requested a review from matbme December 18, 2023 13:31
cmd/pkg.go Show resolved Hide resolved
cmd/upgrade.go Show resolved Hide resolved
Copy link
Member

@kbdharun kbdharun left a comment

Choose a reason for hiding this comment

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

The locale changes LGTM, will lock the component on Weblate to prevent merge conficts after this PRs merge.

@matbme
Copy link
Member

matbme commented Dec 18, 2023

Needs some extra checks to avoid doing destructive operations:

I think a dry-run should enter these functions and skip the write operation. If we modify part-future, the future partition may become unusable and it's not a dry-run anymore

move image removal after dry run
@mirkobrombin
Copy link
Member Author

Needs some extra checks to avoid doing destructive operations:

I think a dry-run should enter these functions and skip the write operation. If we modify part-future, the future partition may become unusable and it's not a dry-run anymore

Following our discussion on Discord, I moved the image removal after the dry run and made the dry run exit earlier, skipping the init partition. For the other sections, ABRoot is writing inside .system.new, not the actual .system folder, so it is fine.

@mirkobrombin mirkobrombin merged commit 7a5fefa into main Dec 18, 2023
4 checks passed
@mirkobrombin mirkobrombin deleted the feat/Add---dry-run-option branch December 18, 2023 14:41
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.

Add --dry-run option
3 participants