Skip to content

Conversation

@askb
Copy link
Contributor

@askb askb commented Mar 21, 2025

"Issue-ID:" keyword is chosen by the TOC/TSC and the format can vary between LF projects, therefore don't hard code this value. The entire string is expected to be passed from inject issue-id job.

Clean up unecessary code.

Copy link
Member

@tykeal tykeal left a comment

Choose a reason for hiding this comment

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

I see two variables being used here, but no documentation as to what is expected.

Given how the two variables are being used I wouldn't know how to set them to get the expected value result of an Issue ID footer being added to the commit. Please add documentation.

@askb
Copy link
Contributor Author

askb commented Mar 24, 2025

I see two variables being used here, but no documentation as to what is expected.

Given how the two variables are being used I wouldn't know how to set them to get the expected value result of an Issue ID footer being added to the commit. Please add documentation.

There is just one var vars.ISSUEID set to true and the inject-issue-id workflow injects the issue-id to be set in SET_ISSUE_ID env var.

@tykeal
Copy link
Member

tykeal commented Mar 25, 2025

I see two variables being used here, but no documentation as to what is expected.
Given how the two variables are being used I wouldn't know how to set them to get the expected value result of an Issue ID footer being added to the commit. Please add documentation.

There is just one var vars.ISSUEID set to true and the inject-issue-id workflow injects the issue-id to be set in SET_ISSUE_ID env var.

Ok, that's great, please update the documentation to reflect all of this. Currently the documentation does not state this it states that SET_ISSUE_ID will be set with Issue-ID: as a prefix to it. I know that this PR is changing that, but you're not updating the documentation here and it's needed!

"Issue-ID:" keyword is chosen by the TOC/TSC and the format can
vary between LF projects, therefore don't hard code this value.
The entire string is expected to be passed from inject issue-id job.

Clean up unecessary code.

Signed-off-by: Anil Belur <abelur@linuxfoundation.org>
@askb
Copy link
Contributor Author

askb commented Mar 26, 2025

I see two variables being used here, but no documentation as to what is expected.
Given how the two variables are being used I wouldn't know how to set them to get the expected value result of an Issue ID footer being added to the commit. Please add documentation.

There is just one var vars.ISSUEID set to true and the inject-issue-id workflow injects the issue-id to be set in SET_ISSUE_ID env var.

Ok, that's great, please update the documentation to reflect all of this. Currently the documentation does not state this it states that SET_ISSUE_ID will be set with Issue-ID: as a prefix to it. I know that this PR is changing that, but you're not updating the documentation here and it's needed!

Done, I have closed the PR #33 once this merged will rebase and update #33

@tykeal tykeal merged commit cc17fd8 into lfit:main Mar 26, 2025
4 checks passed
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.

2 participants