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

Sending signals by closing the channel #17917

Merged
merged 1 commit into from
Apr 15, 2024

Conversation

Iceber
Copy link
Contributor

@Iceber Iceber commented Dec 6, 2022

his is just a small optimization of the convention of using channels as end signal notifications.

Thank you for contributing to Harbor!

Comprehensive Summary of your change

The end signal is usually sent by closing the channel

Issue being fixed

Fixes #(issue)

Please indicate you've done the following:

  • Well Written Title and Summary of the PR
  • Label the PR as needed. "release-note/ignore-for-release, release-note/new-feature, release-note/update, release-note/enhancement, release-note/community, release-note/breaking-change, release-note/docs, release-note/infra, release-note/deprecation"
  • Accepted the DCO. Commits without the DCO will delay acceptance.
  • Made sure tests are passing and test coverage is added if needed.
  • Considered the docs impact and opened a new docs issue or PR with docs changes if needed in website repository.

@Iceber Iceber requested a review from a team as a code owner December 6, 2022 02:21
@codecov
Copy link

codecov bot commented Dec 6, 2022

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 46.03%. Comparing base (b7b8847) to head (1b36c7b).
Report is 134 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main   #17917       +/-   ##
===========================================
- Coverage   67.56%   46.03%   -21.54%     
===========================================
  Files         991      246      -745     
  Lines      109181    13565    -95616     
  Branches     2719     2781       +62     
===========================================
- Hits        73768     6244    -67524     
+ Misses      31449     7004    -24445     
+ Partials     3964      317     -3647     
Flag Coverage Δ
unittests 46.03% <ø> (-21.54%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 1237 files with indirect coverage changes

@github-actions
Copy link

This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days.

@github-actions github-actions bot added the Stale label Feb 10, 2023
@Vad1mo Vad1mo force-pushed the close_signal_channel branch from 0e8f966 to 9fc0c11 Compare February 10, 2023 15:33
@Vad1mo Vad1mo removed the Stale label Feb 10, 2023
@Vad1mo
Copy link
Member

Vad1mo commented Feb 10, 2023

@Iceber, can you briefly explain what problem your PR solves or addresses, is there an issue related to that?

@github-actions
Copy link

This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days.

@OrlinVasilev
Copy link
Member

Removed the stale
@Iceber can you please explain in brief what is fixing is it a bug or optimisation, looks like optimisation to me :) but still
if no response until the the next stale we have to archive it :(

@Iceber
Copy link
Contributor Author

Iceber commented Apr 12, 2023

@OrlinVasilev Sorry, lots of things going on. This is just a small optimization of the convention of using channels as end signal notifications.

@Iceber Iceber force-pushed the close_signal_channel branch from 6634fe0 to 123846e Compare April 13, 2023 03:33
Copy link
Member

@chlins chlins left a comment

Choose a reason for hiding this comment

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

lgtm

@chlins chlins added the release-note/update Update or Fix label Jun 5, 2023
@github-actions
Copy link

github-actions bot commented Aug 4, 2023

This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days.

@github-actions github-actions bot added the Stale label Aug 4, 2023
@Vad1mo Vad1mo added never-stale Do not stale and removed Stale labels Aug 5, 2023
@chlins chlins force-pushed the close_signal_channel branch from 123846e to 5cc3d91 Compare April 15, 2024 07:42
@chlins chlins enabled auto-merge (squash) April 15, 2024 07:42
@chlins chlins force-pushed the close_signal_channel branch from 5cc3d91 to 1b36c7b Compare April 15, 2024 12:02
@chlins chlins merged commit a2507dc into goharbor:main Apr 15, 2024
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
never-stale Do not stale release-note/update Update or Fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants