Skip to content

Conversation

@1awesomeJ
Copy link
Contributor

@1awesomeJ 1awesomeJ commented Mar 30, 2023

Follow-up for #27020

@github-actions github-actions bot added tests please-review PR is ready for (re-)review by a maintainer labels Mar 30, 2023
@bluca bluca added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed please-review PR is ready for (re-)review by a maintainer labels Mar 30, 2023
Copy link
Collaborator

@DaanDeMeyer DaanDeMeyer left a comment

Choose a reason for hiding this comment

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

Please write a proper commit message

@DaanDeMeyer DaanDeMeyer added reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks and removed good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed labels Mar 31, 2023
@github-actions github-actions bot added please-review PR is ready for (re-)review by a maintainer and removed reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels Mar 31, 2023
@1awesomeJ
Copy link
Contributor Author

Please write a proper commit message

Done sir.

@bluca
Copy link
Member

bluca commented Mar 31, 2023

Please use a more descriptive commit message, it's too generic, check the git log for examples

@bluca bluca added reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks and removed please-review PR is ready for (re-)review by a maintainer labels Mar 31, 2023
@1awesomeJ
Copy link
Contributor Author

Please use a more descriptive commit message, it's too generic, check the git log for examples

Alright sir.

@1awesomeJ
Copy link
Contributor Author

Please use a more descriptive commit message, it's too generic, check the git log for examples

I'm looking at using this commit message:
"fixed 27020 to use in-line error handling rather than redirections"
Is it okay sir?

@bluca
Copy link
Member

bluca commented Mar 31, 2023

test: fix negative checks in TEST-70

Use in-line error handling rather than redirections.

Follow-up for #27020

@1awesomeJ
Copy link
Contributor Author

test: fix negative checks in TEST-70

Use in-line error handling rather than redirections.

Follow-up for #27020

Wow.. Thank you so much sir.

@1awesomeJ 1awesomeJ changed the title TEST-70-TPM2: fixed negative test cases test: fixed negative checks in TEST-70-TPM2 Mar 31, 2023
@github-actions github-actions bot added please-review PR is ready for (re-)review by a maintainer and removed reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels Mar 31, 2023
@1awesomeJ
Copy link
Contributor Author

@DaanDeMeyer @bluca, commit message fixed sirs.

@bluca
Copy link
Member

bluca commented Mar 31, 2023

It's not fixed, you've changed the PR description, but the git commit is still the same

@bluca bluca added reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks and removed please-review PR is ready for (re-)review by a maintainer labels Mar 31, 2023
@1awesomeJ
Copy link
Contributor Author

My old commit message was "removed redirections to grep" sir.

this is what i changed to:
commit_a

@1awesomeJ
Copy link
Contributor Author

Then I added this to the PR descrition at the top:
commit_b

@bluca
Copy link
Member

bluca commented Mar 31, 2023

Yes, the ask is to use a better commit message, like the one I shared, so please amend it and push again

@1awesomeJ
Copy link
Contributor Author

test: fix negative checks in TEST-70

I used this in the PR description.

Use in-line error handling rather than redirections.

This in the commit message

Follow-up for #27020
and this at the top of the conversation sir.

@1awesomeJ
Copy link
Contributor Author

Yes, the ask is to use a better commit message, like the one I shared, so please amend it and push again

Sorry for making this drag for too long sir.
"Use in-line error handling rather than redirections. Follow up on #27020"

right?

@bluca
Copy link
Member

bluca commented Mar 31, 2023

You can see the commit message here:

b26772b

@bluca
Copy link
Member

bluca commented Mar 31, 2023

Yes, the ask is to use a better commit message, like the one I shared, so please amend it and push again

Sorry for making this drag for too long sir. "Use in-line error handling rather than redirections. Follow up on #27020"

right?

Make it match the PR title and description, with the title as the first line

@github-actions github-actions bot added please-review PR is ready for (re-)review by a maintainer and removed reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels Mar 31, 2023
@1awesomeJ
Copy link
Contributor Author

Yes, the ask is to use a better commit message, like the one I shared, so please amend it and push again

Sorry for making this drag for too long sir. "Use in-line error handling rather than redirections. Follow up on #27020"
right?

Make it match the PR title and description, with the title as the first line

I have made the update sir. Thank you for the guidance.
I appreciate the fact that the coding standards are this high, it's being a good experience.

Is the current commit message good to fly? Does it yet need some fixes?

@bluca
Copy link
Member

bluca commented Mar 31, 2023

almost - it's all in a single line, split it after the first period

@1awesomeJ
Copy link
Contributor Author

almost - it's all in a single line, split it after the first period

Awesome.

Use in-line error handling rather than redirections. Follow up on systemd#27020
@1awesomeJ
Copy link
Contributor Author

almost - it's all in a single line, split it after the first period

How about now sir?
Can I say "I've learnt something today"? or I still need to learn a little more?

@bluca bluca added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed please-review PR is ready for (re-)review by a maintainer labels Mar 31, 2023
@bluca bluca merged commit 27d45db into systemd:main Mar 31, 2023
@github-actions github-actions bot removed the good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed label Mar 31, 2023
@1awesomeJ 1awesomeJ deleted the nit branch May 4, 2023 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants