flux-framework / flux-sched

Fluxion Graph-based Scheduler
GNU Lesser General Public License v3.0
89 stars 41 forks source link

Reformat #1218

Closed trws closed 4 months ago

trws commented 5 months ago

Two small tweaks to add this to the format script and remove the special-casing for pack/unpack, then apply formatting. Once we're happy with this, I intend to add a .git-ignore-revs or similar file we can use to ignore the global reformat commit in blame, but it has to have the commit sha in full in the file.

trws commented 5 months ago

Now that the release is together, I rebased the reformat here. Short of adding CI to verify it, which might accidentally work because it's in the format script now that I think about it, this should be good to go.

trws commented 4 months ago

@milroy, what do you think of just getting this in? I've rebased and reapplied the formatting. I've also created a pull request to your rank-based-partial-release branch with a rebase of your branch on top of this with the reformatting applied to every commit to preserve all your changes. If you have new changes that aren't in there, or just want to do it yourself, this script completely automates it assuming you have fetched my reformat branch and are currently on your "partial-release" branch, or any other branch for that matter:

export BEFORE_MASS_FORMAT=af44d3b53457240d511bd2a32a3485fbdb91246d
export MASS_FORMAT=78b80b195bc36186abb438f499adbee5bef4a025
git rebase \
  --strategy-option theirs \
  --empty=drop \
  --exec "$(echo './scripts/format' \
      '|| echo prettier failed its ok; ' \
      'git commit -a --amend --no-verify --no-edit' \
    )" \
  --onto $MASS_FORMAT $BEFORE_MASS_FORMAT;

This is also now enforced in CI in this PR.

trws commented 4 months ago

Apologies if anyone looked at this during the churn after the last comment. Had to switch our format infrastructure to use pre-commit like core does to ensure we get consistent versions and avoid formatting differences from that. Now all straightened up, and using clang-format 18.1.6.

trws commented 4 months ago

Reminder to self: When we're ready to pull the trigger here, I need to change the name of the action in the branch protection rules so this and other PRs with the new name can pass.

milroy commented 4 months ago

Sorry for the delay. I'll review this PR tonight.

milroy commented 4 months ago

I've also created a pull request to your rank-based-partial-release branch with a rebase of your branch on top of this with the reformatting applied to every commit to preserve all your changes. If you have new changes that aren't in there, or just want to do it yourself, this script completely automates it assuming you have fetched my reformat branch and are currently on your "partial-release" branch, or any other branch for that matter

Thanks a lot for providing an automated way to update my PR. I've made a few changes to the branch so I I'll run the command myself.

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 79.58225% with 391 lines in your changes missing coverage. Please review.

Project coverage is 74.4%. Comparing base (fc4fcde) to head (806f8fc). Report is 139 commits behind head on master.

Files with missing lines Patch % Lines
resource/reapi/bindings/c++/reapi_cli_impl.hpp 30.0% 86 Missing :warning:
resource/readers/resource_reader_hwloc.cpp 64.5% 44 Missing :warning:
qmanager/modules/qmanager_callbacks.cpp 59.7% 29 Missing :warning:
resource/libjobspec/jobspec.cpp 85.8% 19 Missing :warning:
qmanager/modules/qmanager_opts.cpp 80.0% 17 Missing :warning:
resource/modules/resource_match_opts.cpp 83.8% 16 Missing :warning:
qmanager/modules/qmanager.cpp 81.8% 14 Missing :warning:
resource/readers/resource_reader_grug.cpp 83.5% 14 Missing :warning:
resource/reapi/bindings/c++/reapi_module_impl.hpp 46.1% 14 Missing :warning:
resource/planner/c++/planner_multi.cpp 57.1% 9 Missing :warning:
... and 30 more
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1218 +/- ## ======================================= Coverage 74.3% 74.4% ======================================= Files 104 104 Lines 15184 14936 -248 ======================================= - Hits 11295 11118 -177 + Misses 3889 3818 -71 ``` | [Files with missing lines](https://app.codecov.io/gh/flux-framework/flux-sched/pull/1218?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flux-framework) | Coverage Δ | | |---|---|---| | [qmanager/modules/qmanager\_opts.hpp](https://app.codecov.io/gh/flux-framework/flux-sched/pull/1218?src=pr&el=tree&filepath=qmanager%2Fmodules%2Fqmanager_opts.hpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flux-framework#diff-cW1hbmFnZXIvbW9kdWxlcy9xbWFuYWdlcl9vcHRzLmhwcA==) | `100.0% <ø> (ø)` | | | [...anager/policies/queue\_policy\_conservative\_impl.hpp](https://app.codecov.io/gh/flux-framework/flux-sched/pull/1218?src=pr&el=tree&filepath=qmanager%2Fpolicies%2Fqueue_policy_conservative_impl.hpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flux-framework#diff-cW1hbmFnZXIvcG9saWNpZXMvcXVldWVfcG9saWN5X2NvbnNlcnZhdGl2ZV9pbXBsLmhwcA==) | `76.0% <100.0%> (-1.0%)` | :arrow_down: | | [qmanager/policies/queue\_policy\_easy\_impl.hpp](https://app.codecov.io/gh/flux-framework/flux-sched/pull/1218?src=pr&el=tree&filepath=qmanager%2Fpolicies%2Fqueue_policy_easy_impl.hpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flux-framework#diff-cW1hbmFnZXIvcG9saWNpZXMvcXVldWVfcG9saWN5X2Vhc3lfaW1wbC5ocHA=) | `100.0% <ø> (ø)` | | | [qmanager/policies/queue\_policy\_factory\_impl.hpp](https://app.codecov.io/gh/flux-framework/flux-sched/pull/1218?src=pr&el=tree&filepath=qmanager%2Fpolicies%2Fqueue_policy_factory_impl.hpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flux-framework#diff-cW1hbmFnZXIvcG9saWNpZXMvcXVldWVfcG9saWN5X2ZhY3RvcnlfaW1wbC5ocHA=) | `84.2% <100.0%> (-0.8%)` | :arrow_down: | | [qmanager/policies/queue\_policy\_fcfs.hpp](https://app.codecov.io/gh/flux-framework/flux-sched/pull/1218?src=pr&el=tree&filepath=qmanager%2Fpolicies%2Fqueue_policy_fcfs.hpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flux-framework#diff-cW1hbmFnZXIvcG9saWNpZXMvcXVldWVfcG9saWN5X2ZjZnMuaHBw) | `100.0% <100.0%> (ø)` | | | [qmanager/policies/queue\_policy\_hybrid\_impl.hpp](https://app.codecov.io/gh/flux-framework/flux-sched/pull/1218?src=pr&el=tree&filepath=qmanager%2Fpolicies%2Fqueue_policy_hybrid_impl.hpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flux-framework#diff-cW1hbmFnZXIvcG9saWNpZXMvcXVldWVfcG9saWN5X2h5YnJpZF9pbXBsLmhwcA==) | `69.2% <100.0%> (-2.2%)` | :arrow_down: | | [resource/evaluators/expr\_eval\_target.hpp](https://app.codecov.io/gh/flux-framework/flux-sched/pull/1218?src=pr&el=tree&filepath=resource%2Fevaluators%2Fexpr_eval_target.hpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flux-framework#diff-cmVzb3VyY2UvZXZhbHVhdG9ycy9leHByX2V2YWxfdGFyZ2V0LmhwcA==) | `100.0% <ø> (ø)` | | | [resource/evaluators/expr\_eval\_vtx\_target.cpp](https://app.codecov.io/gh/flux-framework/flux-sched/pull/1218?src=pr&el=tree&filepath=resource%2Fevaluators%2Fexpr_eval_vtx_target.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flux-framework#diff-cmVzb3VyY2UvZXZhbHVhdG9ycy9leHByX2V2YWxfdnR4X3RhcmdldC5jcHA=) | `89.4% <100.0%> (-0.4%)` | :arrow_down: | | [resource/evaluators/expr\_eval\_vtx\_target.hpp](https://app.codecov.io/gh/flux-framework/flux-sched/pull/1218?src=pr&el=tree&filepath=resource%2Fevaluators%2Fexpr_eval_vtx_target.hpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flux-framework#diff-cmVzb3VyY2UvZXZhbHVhdG9ycy9leHByX2V2YWxfdnR4X3RhcmdldC5ocHA=) | `100.0% <ø> (ø)` | | | [resource/evaluators/test/expr\_eval\_test01.cpp](https://app.codecov.io/gh/flux-framework/flux-sched/pull/1218?src=pr&el=tree&filepath=resource%2Fevaluators%2Ftest%2Fexpr_eval_test01.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flux-framework#diff-cmVzb3VyY2UvZXZhbHVhdG9ycy90ZXN0L2V4cHJfZXZhbF90ZXN0MDEuY3Bw) | `100.0% <100.0%> (ø)` | | | ... and [91 more](https://app.codecov.io/gh/flux-framework/flux-sched/pull/1218?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flux-framework) | |