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

Remove deprecations in Harmonic #426

Merged
merged 5 commits into from
Aug 14, 2023
Merged

Conversation

caguero
Copy link
Collaborator

@caguero caguero commented Jul 28, 2023

🎉 Maintenance

Remove deprecations

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

nkoenig and others added 3 commits July 27, 2023 05:45
Signed-off-by: Nate Koenig <[email protected]>
Signed-off-by: Nate Koenig <[email protected]>
Signed-off-by: Carlos Agüero <[email protected]>
@azeey azeey added the beta Targeting beta release of upcoming collection label Jul 31, 2023
@mjcarroll mjcarroll changed the base branch from nkoenig/remove-ign to main August 4, 2023 13:54
@codecov
Copy link

codecov bot commented Aug 4, 2023

Codecov Report

Merging #426 (fbd8a01) into main (3a11524) will increase coverage by 0.35%.
The diff coverage is 100.00%.

❗ Current head fbd8a01 differs from pull request most recent head e5d3ef1. Consider uploading reports for the commit e5d3ef1 to get more accurate results

@@            Coverage Diff             @@
##             main     #426      +/-   ##
==========================================
+ Coverage   87.40%   87.75%   +0.35%     
==========================================
  Files          59       59              
  Lines        5747     5693      -54     
==========================================
- Hits         5023     4996      -27     
+ Misses        724      697      -27     
Files Changed Coverage Δ
include/gz/transport/Discovery.hh 86.95% <ø> (+0.94%) ⬆️
log/src/Log.cc 79.45% <ø> (+0.73%) ⬆️
src/NetUtils.cc 75.22% <ø> (+1.12%) ⬆️
src/NodeOptions.cc 100.00% <ø> (+3.70%) ⬆️
src/NodeShared.cc 78.99% <100.00%> (+0.95%) ⬆️

... and 1 file with indirect coverage changes

@mjcarroll
Copy link
Contributor

@osrf-jenkins retest this please

@caguero
Copy link
Collaborator Author

caguero commented Aug 11, 2023

@Crola1702 , do you know why the Ubuntu Focal job gets stuck?

@Crola1702
Copy link
Contributor

Crola1702 commented Aug 11, 2023

I think for harmonic we're not supporting Focal but Jammy, right? Maybe it's triggered as a Jammy job, but checks are expecting a Focal job 🤔

@Crola1702
Copy link
Contributor

Maybe @j-rivero has more context on this?

@caguero
Copy link
Collaborator Author

caguero commented Aug 11, 2023

I think for harmonic we're not supporting Focal but Jammy, right? Maybe it's triggered as a Jammy job, but checks are expecting a Focal job thinking

As far as I know it's only Jammy (and the future 24.04). Not sure why it's triggered...

@mjcarroll
Copy link
Contributor

As far as I know it's only Jammy (and the future 24.04). Not sure why it's triggered...

It's because this PR was opened before we disabled the focal job for the target branch. It can safely be ignored.

@caguero
Copy link
Collaborator Author

caguero commented Aug 11, 2023

As far as I know it's only Jammy (and the future 24.04). Not sure why it's triggered...

It's because this PR was opened before we disabled the focal job for the target branch. It can safely be ignored.

I see. I don't have superpowers to merge this PR without all the checks passing...

@mjcarroll
Copy link
Contributor

Is the failing windows test okay?

@mjcarroll
Copy link
Contributor

I see. I don't have superpowers to merge this PR without all the checks passing...

I removed the branch protection check, can you merge it now?

@caguero caguero merged commit d888517 into main Aug 14, 2023
6 checks passed
@caguero caguero deleted the caguero_deprecations_harmonic branch August 14, 2023 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta Targeting beta release of upcoming collection
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants