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

flux-ion-resource: move to src/cmd and install #1127

Merged
merged 2 commits into from
Dec 22, 2023

Conversation

trws
Copy link
Member

@trws trws commented Dec 22, 2023

problem: recent production debugging and fixes have benefitted from having flux ion-resource available, but it is not installed.

solution: install it, and since it's no longer a test-only script, move it to src/cmd/

fixes #1122

@trws
Copy link
Member Author

trws commented Dec 22, 2023

Ok, this should be good to go now I hope. It's notable that the first commit, the one that's a move and one line, is all it takes to install the script. The second commit is required for flux-ion-resource to pass pylint, it was in pretty tough shape and this was the minimum to make it pass (almost, bit of extra cleanup too).

@trws trws requested a review from grondo December 22, 2023 20:50
@trws trws force-pushed the install-flux-ion-resource branch from c4b7b6d to 8159961 Compare December 22, 2023 20:51
trws added 2 commits December 22, 2023 12:52
problem: recent production debugging and fixes have benefitted from
having flux ion-resource available, but it is not installed.

solution: install it, and since it's no longer a test-only script, move
it to src/cmd/
problem: pylint is strict, and this script was never checked because it
was in the test directory.

solution: Commit all necessary sacrifices and rites to pylint the great
and merciless.
@trws trws force-pushed the install-flux-ion-resource branch from 8159961 to e4fd996 Compare December 22, 2023 20:52
Copy link
Contributor

@grondo grondo left a comment

Choose a reason for hiding this comment

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

LGTM! I verified that the make deb target works and now includes flux ion-resource (and that the script is functional)

Thanks!

@trws trws added the merge-when-passing mergify.io - merge PR automatically once CI passes label Dec 22, 2023
Copy link

codecov bot commented Dec 22, 2023

Codecov Report

Merging #1127 (e4fd996) into master (745e3e0) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files
@@          Coverage Diff           @@
##           master   #1127   +/-   ##
======================================
  Coverage    70.1%   70.1%           
======================================
  Files          90      90           
  Lines       12264   12264           
======================================
  Hits         8605    8605           
  Misses       3659    3659           

@mergify mergify bot merged commit d51c81f into flux-framework:master Dec 22, 2023
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge-when-passing mergify.io - merge PR automatically once CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

install and package flux ion-resource
2 participants