ros2 / sros2

tools to generate and distribute keys for SROS 2
Apache License 2.0
88 stars 43 forks source link

remove deprecated create_key and list_keys verbs #302

Closed mikaelarguedas closed 2 months ago

mikaelarguedas commented 2 months ago

Deprecated since 2020

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 77.35%. Comparing base (554d1c7) to head (02bc4f1).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## rolling #302 +/- ## =========================================== + Coverage 77.08% 77.35% +0.27% =========================================== Files 25 25 Lines 637 627 -10 Branches 66 66 =========================================== - Hits 491 485 -6 + Misses 125 121 -4 Partials 21 21 ``` | [Flag](https://app.codecov.io/gh/ros2/sros2/pull/302/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros2) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/ros2/sros2/pull/302/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros2) | `77.35% <ø> (+0.27%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros2#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

clalancette commented 2 months ago

These kinds of changes need to go through a CI round in order to get merged into Rolling, as per our Contributor guidelines.

Please make sure to run CI on these changes, and for any future PRs.

mikaelarguedas commented 2 months ago

Oh my bad I assumed that the approval from @ahcorde was enough (as there is no instance of these verbs in the entire codebase).

will do in the future :+1:

We agree that that doesnt apply to PRs for local CI purposes like my ongoing github actions and codecov PRs?

clalancette commented 2 months ago

We agree that that doesnt apply to PRs for local CI purposes like my ongoing github actions and codecov PRs?

Yeah, those are totally fine; they don't affect the code directly.

mikaelarguedas commented 2 months ago

Yeah, those are totally fine; they don't affect the code directly.

Neither does removing verbs that were never used anywhere in the ros2.repos file :thinking:

clalancette commented 2 months ago

Neither does removing verbs that were never used anywhere in the ros2.repos file 🤔

Nevertheless, it is our development practice to always run CI on code changes. There have been very many times when a change that should not have changed anything broke on Windows.

mikaelarguedas commented 2 months ago

ok :+1:

Which jobs should be ran in debug mode ?

I dont see it in the developper workflow and from my recollection that's where most of my python change headaches came from (Python + Debug + Windows)

mikaelarguedas commented 2 months ago

(not trying to be sassy, just wanting to make sure I follow the right workflow for https://github.com/ros2/sros2/pull/300 and future ones)

clalancette commented 2 months ago

Which jobs should be ran in debug mode ?

Yeah, it's a good question. The honest answer is that we don't (typically) run Windows Debug as part of normal CI. We usually just wait for the nightlies to run it, and then fixup afterwards.

In any case, it looks like you've figure out how to run it, so that is a nice bonus. Thanks for doing that.