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

Delete deprecated bench-scripts from agent Makefile #2802

Merged
merged 1 commit into from
Apr 29, 2022

Conversation

ndokos
Copy link
Member

@ndokos ndokos commented Apr 29, 2022

Fixes #2801

PR #2782 deleted deprecated bench scripts from the tree, but did
not delete them from the Makefile.

@ndokos ndokos added this to the v0.72 milestone Apr 29, 2022
@ndokos ndokos requested a review from portante April 29, 2022 18:17
@ndokos ndokos self-assigned this Apr 29, 2022
webbnh
webbnh previously approved these changes Apr 29, 2022
Copy link
Member

@webbnh webbnh left a comment

Choose a reason for hiding this comment

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

If you don't mind a little feature creep, pyproject.toml needs updating, too, for a similar reason. (The references to binary-search.py and profile-builder.py should be removed from Black's exclusions list, and dbench-postprocess should be removed from the inclusion list.)

dbutenhof
dbutenhof previously approved these changes Apr 29, 2022
@ndokos
Copy link
Member Author

ndokos commented Apr 29, 2022

@webbnh: I don't mind the feature creep but dbench-postprocess still exists in the tree (presumably it needs to go), so I presume I should delete it altogether (there is also a mocked version under test-bin).

@webbnh
Copy link
Member

webbnh commented Apr 29, 2022

I'll defer to @portante.... 😉

@ndokos ndokos dismissed stale reviews from webbnh and dbutenhof via a82ea10 April 29, 2022 18:53
portante
portante previously approved these changes Apr 29, 2022
@ndokos
Copy link
Member Author

ndokos commented Apr 29, 2022

I pushed the changes above (and more) and I'll wait for @portante's approval (which came while I was typing...)

@ndokos ndokos requested review from webbnh and dbutenhof April 29, 2022 18:56
dbutenhof
dbutenhof previously approved these changes Apr 29, 2022
Copy link
Member

@dbutenhof dbutenhof left a comment

Choose a reason for hiding this comment

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

🥳

webbnh
webbnh previously approved these changes Apr 29, 2022
Copy link
Member

@webbnh webbnh left a comment

Choose a reason for hiding this comment

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

Yay!...but....

Fixes distributed-system-analysis#2801

PR distributed-system-analysis#2782 deleted deprecated bench scripts from the tree, but did
not delete them from the Makefile.

Additional problems were uncovered in code review:

- pyproject.toml contained a couple of references to
no-longer-existing files and a reference to dbench-postprocess.

- dbench-postprocess is no longer necessary and is deleted (both
the "real" version and the mocked test version).

- pbench-agent-default.cfg still contained references to the deleted
bench-scripts, so it is cleaned up.

docs/pbench-agent.org still mentions dbench (and maybe other deleted
bench-scripts) but I declared it out of scope for this PR. We'll take
care of it in the documentation cleanup.
@ndokos ndokos dismissed stale reviews from webbnh, dbutenhof, and portante via 010ae58 April 29, 2022 19:11
@ndokos ndokos requested review from portante, webbnh and dbutenhof April 29, 2022 19:20
Copy link
Member

@webbnh webbnh left a comment

Choose a reason for hiding this comment

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

🚀

@portante
Copy link
Member

See also PR #2803.

@ndokos ndokos merged commit e5e8855 into distributed-system-analysis:main Apr 29, 2022
@ndokos ndokos deleted the pr-2782-addon branch April 29, 2022 20:16
@portante portante added packaging Issues related to software packaging Backport PRs with this label should be considered for back porting to previous release branches and removed Backport PRs with this label should be considered for back porting to previous release branches labels Apr 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Agent bug packaging Issues related to software packaging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] PR #2782 removed deprecated bench scripts but did not fix the agent Makefile to remove them
4 participants