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.

Sign up for GitHub

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

Jump to bottom

Support --workdir-root in the tmt clean command #2850

Merged
merged 17 commits into from
Nov 27, 2024
Merged

Support --workdir-root in the tmt clean command #2850

merged 17 commits into from
Nov 27, 2024

Conversation

skycastlelily
Copy link
Collaborator

@skycastlelily skycastlelily commented Apr 12, 2024 •
edited
Loading

Pull Request Checklist

  • implement the feature
  • extend the test coverage
  • include a release note

@skycastlelily skycastlelily mentioned this pull request Apr 12, 2024
Open
happz
happz reviewed Apr 16, 2024
View reviewed changes
tmt/base.py Outdated Show resolved Hide resolved
@skycastlelily skycastlelily changed the title Accept CLI input in get_effective_root , and have Clean use it. Accept CLI input in effective_workdir_root , and have Clean use it. Apr 16, 2024
@skycastlelily skycastlelily force-pushed the effective branch 2 times, most recently from 1f8d67b to 8bd2d7f Compare April 24, 2024 01:19
@skycastlelily skycastlelily force-pushed the effective branch 2 times, most recently from 8c8da09 to 8d9a62a Compare August 22, 2024 08:37
@skycastlelily skycastlelily mentioned this pull request Aug 22, 2024
Open
@skycastlelily
Copy link
Collaborator Author

Hi Petr, @psss, would you please spare some time to review this merge request,please? This is supposed to address #2807 , sorry for forgetting mentioned it before 1.36 release,and hope we can have this merged for 1.37: )

@skycastlelily skycastlelily added this to the 1.37 milestone Sep 25, 2024
@skycastlelily skycastlelily added the ci | full test Pull request is ready for the full test execution label Sep 25, 2024
@thrix thrix requested a review from happz September 26, 2024 11:36
thrix
thrix reviewed Sep 26, 2024
View reviewed changes
docs/releases.rst Outdated Show resolved Hide resolved
thrix
thrix reviewed Sep 26, 2024
View reviewed changes
docs/releases.rst Outdated Show resolved Hide resolved
martinhoyer
martinhoyer reviewed Sep 26, 2024
View reviewed changes
Copy link
Collaborator

@martinhoyer martinhoyer left a comment

Choose a reason for hiding this comment

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

lgtm. Just nitpicks.
I'm surprised setting a workdir root is so complicated tbh (not necessarily related to these changes). :)

tmt/cli.py Outdated Show resolved Hide resolved
tmt/utils/__init__.py Outdated Show resolved Hide resolved
@thrix
Copy link
Collaborator

thrix commented Sep 26, 2024

@skycastlelily would it be possible to log the workdir root please in case the verbose mode is enabled please?

Something like:

❯ env | grep WORKDIR
TMT_WORKDIR_ROOT=/var/home/mvadkert/.local/share/tmt

❯ tmt clean -v
clean
    workdir /var/home/mvadkert/.local/share/tmt
guests
runs
images
    testcloud

happz
happz reviewed Sep 26, 2024
View reviewed changes
tmt/base.py Outdated Show resolved Hide resolved
happz
happz reviewed Sep 26, 2024
View reviewed changes
tmt/base.py Outdated Show resolved Hide resolved
happz
happz reviewed Sep 26, 2024
View reviewed changes
tmt/utils/__init__.py Outdated Show resolved Hide resolved
happz
happz reviewed Sep 26, 2024
View reviewed changes
tmt/options.py Outdated Show resolved Hide resolved
psss
psss requested changes Nov 22, 2024
View reviewed changes
tmt/cli/_root.py Outdated Show resolved Hide resolved
@psss psss removed the status | ready for merge The only missing piece is to do the rebase the current 'main' and let the CI finish. label Nov 22, 2024
@skycastlelily
Copy link
Collaborator Author

Sorry for that. I was ready to include it in the release but tests were red. And we cannot postpone the release indefinitely. Let's merge the pull request as one of the first ones in 1.40 to prevent any further rebase problems.

Make sense,and sounds great :)

psss
psss approved these changes Nov 27, 2024
View reviewed changes
Copy link
Collaborator

@psss psss left a comment

Choose a reason for hiding this comment

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

Now looks good, thanks!

@skycastlelily
Copy link
Collaborator Author

Now looks good, thanks!

Pleasure,I guess we could finally have this one merged? 🙂

@psss
Copy link
Collaborator

psss commented Nov 27, 2024

Yes, just rebased on the latest main to get rid of those irrelevant test failures.

@psss psss added the status | ready for merge The only missing piece is to do the rebase the current 'main' and let the CI finish. label Nov 27, 2024
@psss
Copy link
Collaborator

psss commented Nov 27, 2024

Red tests are irrelevant failures, merging.

@psss psss merged commit b67fa54 into teemtee:main Nov 27, 2024
18 of 20 checks passed
@psss psss self-assigned this Nov 27, 2024
@skycastlelily
Copy link
Collaborator Author

skycastlelily commented Nov 28, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thrix thrix thrix left review comments

@happz happz happz approved these changes

@therazix therazix therazix approved these changes

@psss psss psss approved these changes

@KwisatzHaderach KwisatzHaderach KwisatzHaderach approved these changes

@lukaszachy lukaszachy Awaiting requested review from lukaszachy lukaszachy is a code owner

@janhavlin janhavlin Awaiting requested review from janhavlin janhavlin is a code owner

@frantisekz frantisekz Awaiting requested review from frantisekz

@martinhoyer martinhoyer Awaiting requested review from martinhoyer

Assignees

@psss psss

Labels
ci | full test Pull request is ready for the full test execution code | cli Changes related to the command line interface command | clean tmt clean command status | ready for merge The only missing piece is to do the rebase the current 'main' and let the CI finish.
Projects
None yet
Milestone
1.40
Development

Successfully merging this pull request may close these issues.
7 participants
@skycastlelily @thrix @happz @psss @martinhoyer @KwisatzHaderach @therazix