Skip to content

Conversation

chethanm99
Copy link
Contributor

@chethanm99 chethanm99 commented Jun 27, 2025

Thank you for contributing to Harbor!

Comprehensive Summary of your change

Issue being fixed

Fixes goharbor/website#666

This PR fixes the issue and improves the user experience the other file changes related to this PR is here : goharbor/website#658

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.

Copy link

codecov bot commented Jun 27, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.56%. Comparing base (c8c11b4) to head (8e01678).
Report is 498 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main   goharbor/harbor#22128       +/-   ##
===========================================
+ Coverage   45.36%   68.56%   +23.19%     
===========================================
  Files         244      819      +575     
  Lines       13333   101462    +88129     
  Branches     2719        0     -2719     
===========================================
+ Hits         6049    69569    +63520     
- Misses       6983    28019    +21036     
- Partials      301     3874     +3573     
Flag Coverage Δ
unittests 68.56% <ø> (+23.19%) ⬆️

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

see 1051 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

# (e.g., Docker CLI, web browsers) support modern TLS 1.2+ ciphers to avoid connection issues.
# enable strong ssl ciphers (default: false)
# strong_ssl_ciphers: false
strong_ssl_ciphers: true
Copy link
Member

Choose a reason for hiding this comment

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

are you improving the docs or changing the logic?
It seems like you changed from false to true # enable strong ssl ciphers (default: false)
I suggest only doing one thing in the PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure I will rectify the logic part and let this PR only focus on adding comments. Thanks for notifying

@chethanm99 chethanm99 force-pushed the feat/add-comments-to-yml branch 3 times, most recently from 1cd8c76 to 94ca49c Compare June 27, 2025 12:47
Signed-off-by: chethanm99 <chethanm1399@gmail.com>
@chethanm99 chethanm99 force-pushed the feat/add-comments-to-yml branch from 94ca49c to 8e01678 Compare June 27, 2025 12:47
@chethanm99
Copy link
Contributor Author

@Vad1mo I've made changes , please let me know if any other changes to be made.

Copy link
Contributor

@bupd bupd left a comment

Choose a reason for hiding this comment

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

This might be an overkill. the things are pretty self explanatory already IMO.

@chethanm99 chethanm99 closed this Jun 27, 2025
@chethanm99
Copy link
Contributor Author

@bupd I understand your point, since you are telling that this change isn't necessary and wouldn't help much, let me just go ahead and close this PR.

Thanks for your time !

@chethanm99 chethanm99 deleted the feat/add-comments-to-yml branch June 28, 2025 04:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suggested Documentation Improvements
6 participants