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

DAOS-623 packaging: Fix go/golang per distro name #12588

Merged
merged 3 commits into from
Jul 11, 2023
Merged

DAOS-623 packaging: Fix go/golang per distro name #12588

merged 3 commits into from
Jul 11, 2023

Conversation

brianjmurrell
Copy link
Contributor

@brianjmurrell brianjmurrell commented Jul 6, 2023 •
edited
Loading

In the Requires: on the client-tests

Before requesting gatekeeper:

  • Two review approvals and any prior change requests have been resolved.
  • Testing is complete and all tests passed or there is a reason documented in the PR why it should be force landed and forced-landing tag is set.
  • Features: (or Test-tag*) commit pragma was used or there is a reason documented that there are no appropriate tags for this PR.
    • Test-tag: test_dfuse_daos_build_wb was used to validate that the daos build test passes
  • Commit messages follows the guidelines outlined here.
  • Any tests skipped by the ticket being addressed have been run and passed in the PR.

Gatekeeper:

  • You are the appropriate gatekeeper to be landing the patch.
  • The PR has 2 reviews by people familiar with the code, including appropriate watchers.
  • Githooks were used. If not, request that user install them and check copyright dates.
  • Checkpatch issues are resolved. Pay particular attention to ones that will show up on future PRs.
  • All builds have passed. Check non-required builds for any new compiler warnings.
  • Sufficient testing is done. Check feature pragmas and test tags and that tests skipped for the ticket are run and now pass with the changes.
  • If applicable, the PR has addressed any potential version compatibility issues.
  • Check the target branch. If it is master branch, should the PR go to a feature branch? If it is a release branch, does it have merge approval in the JIRA ticket.
  • Extra checks if forced landing is requested
    • Review comments are sufficiently resolved, particularly by prior reviewers that requested changes.
    • No new NLT or valgrind warnings. Check the classic view.
    • Quick-build or Quick-functional is not used.
  • Fix the commit message upon landing. Check the standard here. Edit it to create a single commit. If necessary, ask submitter for a new summary.

@brianjmurrell brianjmurrell self-assigned this Jul 6, 2023
@github-actions
Copy link

github-actions bot commented Jul 6, 2023

Bug-tracker data:
Ticket title is 'Generic ticket for minor code cleanup and improvement'
Status is 'Resolved'
Errors are Unknown component
https://daosio.atlassian.net/browse/DAOS-623

daosbuild1
daosbuild1 reviewed Jul 6, 2023
View reviewed changes
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

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

LGTM. No errors found by checkpatch.

daltonbohning
daltonbohning reviewed Jul 6, 2023
View reviewed changes
Copy link
Contributor

@daltonbohning daltonbohning 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 this. I just hit this exact problem on Aurora! Does this not need a release bump and changelog?

@brianjmurrell
Copy link
Contributor Author

brianjmurrell commented Jul 6, 2023 •
edited
Loading

Thanks for this. I just hit this exact problem on Aurora! Does this not need a release bump and changelog?

It does, once I have seen that it's successful and I am ready for it to be reviewed and landed. Updating those pre-maturely typically means having to continually resolve conflicts and update them every time you rebase/merge. Waiting until you are ready for reviews means just having to set them once.

But I also need to add the test tag that does the daos build test.

daosbuild1
daosbuild1 reviewed Jul 6, 2023
View reviewed changes
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

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

LGTM. No errors found by checkpatch.

@brianjmurrell brianjmurrell force-pushed the bmurrell/fix-go-dependency branch 4 times, most recently from cfcb1f6 to bb75707 Compare July 6, 2023 19:53
@ashleypittman
Copy link
Contributor

Can you update the component in the PR title.

@brianjmurrell
Copy link
Contributor Author

brianjmurrell commented Jul 7, 2023 •
edited
Loading

Can you update the component in the PR title.

I would suggest that packaging should be a very valid component. That's exactly what is being updated here. Nothing else.

Do I just add it to

daos/ci/jira_query.py

Lines 30 to 31 in 6e55d30

VALID_COMPONENTS = ('agent', 'build', 'ci', 'csum', 'doc', 'gha', 'il', 'md', 'mercury', 'pil4dfs',
'swim', 'test', 'tools')
or is there something else that uses these component names?

@ashleypittman
Copy link
Contributor

Can you update the component in the PR title.

I would suggest that packaging should be a very valid component. That's exactly what is being updated here. Nothing else.

Do I just add it to

daos/ci/jira_query.py

Lines 30 to 31 in 6e55d30

VALID_COMPONENTS = ('agent', 'build', 'ci', 'csum', 'doc', 'gha', 'il', 'md', 'mercury', 'pil4dfs',
'swim', 'test', 'tools')
or is there something else that uses these component names?

Yes, add it to there if you think it's valid. The check is run using the target, not the pr so this pr will still warn about it but future ones should be happy.

@brianjmurrell
DAOS-623 packaging: Fix go/golang per distro name…
5ecacc1 
In the Requires: on the client-tests

Skip-build-ubuntu20-rpm: true
Skip-unit-tests: true
Skip-func-test-leap15: false
Skip-func-test-el8: false
Skip-func-hw-test: true
Test-tag: test_dfuse_daos_build_wb
Skip-PR-comments: true

Signed-off-by: Brian J. Murrell <brian.murrell@intel.com>
daosbuild1
daosbuild1 reviewed Jul 7, 2023
View reviewed changes
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

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

LGTM. No errors found by checkpatch.

@brianjmurrell brianjmurrell marked this pull request as ready for review July 7, 2023 20:28
@brianjmurrell brianjmurrell requested a review from a team as a code owner July 7, 2023 20:28
daltonbohning
daltonbohning previously approved these changes Jul 7, 2023
View reviewed changes
utils/rpms/daos.spec Outdated
@@ -554,7 +554,10 @@ getent passwd daos_agent >/dev/null || useradd -s /sbin/nologin -r -g daos_agent
# No files in a shim package

%changelog
* Thu Jun 29 2023 Michael MacDonald <mjmac.macdonald@intel.com> 2.3.105-6
* Fri Jul 07 2023 Brian J. Murrell <brian.murrell@intel.com> 2.5.100-7
- Fix golang aos-client-tests dependency to be go instead
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Fix golang aos-client-tests dependency to be go instead
- Fix golang daos-client-tests dependency to be go instead

Copy link
Contributor

Choose a reason for hiding this comment

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

I've updated the PR with this change.

@ashleypittman
Fix spelling of package name.
1750a71 
Skip-build-ubuntu20-rpm: true
Skip-unit-tests: true
Skip-func-test-leap15: false
Skip-func-test-el8: false
Skip-func-hw-test: true
Test-tag: test_dfuse_daos_build_wb
Skip-PR-comments: true
Required-githooks: true

Signed-off-by: Ashley Pittman <ashley.m.pittman@intel.com>
daosbuild1
daosbuild1 reviewed Jul 10, 2023
View reviewed changes
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

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

LGTM. No errors found by checkpatch.

@ashleypittman
Merge branch 'master' into bmurrell/fix-go-dependency
e2154b0 
Skip-func-test-leap15: false

Signed-off-by: Ashley Pittman <ashley.m.pittman@intel.com>
daosbuild1
daosbuild1 reviewed Jul 11, 2023
View reviewed changes
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

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

LGTM. No errors found by checkpatch.

@ashleypittman ashleypittman requested a review from a team July 11, 2023 12:56
@ashleypittman ashleypittman requested a review from a team July 11, 2023 13:01
@gnailzenh gnailzenh merged commit 2e1d372 into master Jul 11, 2023
@gnailzenh gnailzenh deleted the bmurrell/fix-go-dependency branch July 11, 2023 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daosbuild1 daosbuild1 daosbuild1 left review comments

@daltonbohning daltonbohning daltonbohning approved these changes

@gnailzenh gnailzenh gnailzenh approved these changes

@ashleypittman ashleypittman ashleypittman approved these changes

Assignees

@brianjmurrell brianjmurrell

Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@brianjmurrell @ashleypittman @gnailzenh @daltonbohning @daosbuild1