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

test/system: Use env var for invoking Toolbox #777

Merged

Conversation

HarryMichal
Copy link
Member

The system test refactor[0] replaced the 'run_toolbox' helper function
with 'run toolbox', which is a normal invocation of Toolbox. This makes
it impossible to override Toolbox used during the tests using env var.

[0] #693

The system test refactor[0] replaced the 'run_toolbox' helper function
with 'run toolbox', which is a normal invocation of Toolbox. This makes
it impossible to override Toolbox used during the tests using env var.

[0] containers#693
@HarryMichal HarryMichal added 6. Minor Change Should not cause breakage 3. Bugfix Fixes a bug 2. Testing Related to testing of Toolbox - unit, system,.. labels May 26, 2021
@softwarefactory-project-zuul
Copy link

Build failed.

@HarryMichal
Copy link
Member Author

recheck

@softwarefactory-project-zuul
Copy link

Build succeeded.

@HarryMichal HarryMichal merged commit 871d905 into containers:main May 26, 2021
@HarryMichal HarryMichal deleted the test/system/toolbox-env-var branch May 26, 2021 20:52
Copy link
Member

@debarshiray debarshiray left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning this up!

The commit message is missing a link to this PR, though. It's possible to find it by looking up the commit in GitHub's web interface, but that's one extra step that starts to become a hassle after some time.

Also, when referring to other commits in the commit message, it's convenient to include the actual commit ID. Only having the link to the pull request or the issue makes one go through a few more hoops to look at the commit - open the PR or issue in the browser, and look at the commit in the GitHub UI. Given that the reader was looking at the original commit in the terminal, having to switch back and forth between the terminal and browser becomes annoying - the GitHub UI and raw Git formats the commits differently, etc.. It's not impossible but the minor irrations begin to add up. :)

Here are some patterns that I find a bit more convenient:

In short, we write the commit message only once; but in case someone looks it up later, that's usually because the person is either trying to quickly audit the changes in a new release or hunting a bug, and in those cases it's really worth taking the extra trouble to make it easy for the reader.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. Testing Related to testing of Toolbox - unit, system,.. 3. Bugfix Fixes a bug 6. Minor Change Should not cause breakage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants