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

Allow tmt try IP@connect #2991

Merged
merged 2 commits into from
Jul 30, 2024
Merged

Allow tmt try IP@connect #2991

merged 2 commits into from
Jul 30, 2024

Conversation

lukaszachy
Copy link
Collaborator

@lukaszachy lukaszachy commented Jun 5, 2024

Pull Request Checklist

  • implement the feature
  • adjust plugin docstring

@lukaszachy
Copy link
Collaborator Author

lukaszachy commented Jun 5, 2024

@psss how should be this documented? Having to change all IMAGE@HOW into IMAGE_OR_IP@HOW doesn't seem a good idea to me as it will just be confusing.

EDIT: I went with --help example only.

@lukaszachy lukaszachy marked this pull request as ready for review June 11, 2024 12:32
@lukaszachy lukaszachy changed the title Allow `tmt try IP@connect Allow tmt try IP@connect Jun 11, 2024
@lukaszachy lukaszachy added this to the 1.35 milestone Jun 11, 2024
psss
psss previously requested changes Jun 25, 2024
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.

Looks good, I'd just avoid the abbreviation.

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, works as expected.

@martinhoyer martinhoyer requested review from psss and thrix July 30, 2024 09:03
@lukaszachy lukaszachy dismissed psss’s stale review July 30, 2024 09:06

fqdn is the proper term while (unfortunatelly) hostname isn't

@lukaszachy lukaszachy enabled auto-merge (squash) July 30, 2024 09:07
@lukaszachy lukaszachy added the status | ready for merge The only missing piece is to do the rebase the current 'main' and let the CI finish. label Jul 30, 2024
@happz happz disabled auto-merge July 30, 2024 17:15
@happz happz added the ci | full test Pull request is ready for the full test execution label Jul 30, 2024
@happz
Copy link
Collaborator

happz commented Jul 30, 2024

Unrelated failures, merging.

@happz happz merged commit bb1e5d2 into main Jul 30, 2024
19 of 20 checks passed
@happz happz deleted the try-connect branch July 30, 2024 19:18
The-Mule pushed a commit to The-Mule/tmt that referenced this pull request Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci | full test Pull request is ready for the full test execution command | try tmt try 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
Development

Successfully merging this pull request may close these issues.

8 participants