Skip to content

Conversation

KN4CK3R
Copy link
Member

@KN4CK3R KN4CK3R commented Dec 29, 2022

Related to #22262 (comment)

@KN4CK3R KN4CK3R added type/refactoring Existing code has been cleaned up. There should be no new functionality. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. topic/packages labels Dec 29, 2022
@KN4CK3R KN4CK3R added this to the 1.19.0 milestone Dec 29, 2022
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Dec 29, 2022
@delvh delvh self-requested a review December 29, 2022 16:26
@codecov-commenter
Copy link

codecov-commenter commented Dec 29, 2022

Codecov Report

❗ No coverage uploaded for pull request base (main@b76970f). Click here to learn what that means.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##             main   #22268   +/-   ##
=======================================
  Coverage        ?   48.14%           
=======================================
  Files           ?     1043           
  Lines           ?   142380           
  Branches        ?        0           
=======================================
  Hits            ?    68550           
  Misses          ?    65645           
  Partials        ?     8185           
Impacted Files Coverage Δ
modules/packages/composer/metadata.go 75.40% <ø> (ø)
modules/packages/helm/metadata.go 34.88% <ø> (ø)
modules/packages/npm/creator.go 84.95% <ø> (ø)
modules/packages/nuget/metadata.go 86.25% <ø> (ø)
modules/packages/nuget/symbol_extractor.go 54.54% <ø> (ø)
modules/packages/pub/metadata.go 74.02% <ø> (ø)
modules/packages/rubygems/marshal.go 47.59% <ø> (ø)
modules/packages/rubygems/metadata.go 73.25% <ø> (ø)
routers/api/packages/composer/composer.go 72.30% <0.00%> (ø)
routers/api/packages/helm/helm.go 60.24% <0.00%> (ø)
... and 4 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Contributor

@zeripath zeripath left a comment

Choose a reason for hiding this comment

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

I have a feeling we need to make a NewInvalidError function in util,

// NewInvalidError returns an error that formats as the given text but unwraps as an InvalidError
func NewInvalidError(message string) error {
  return SilentWrap{Message: message, Err: util.ErrInvalidArgument}
}

As an aside lots of the comments on the errors have gone missing in this PR.

@zeripath
Copy link
Contributor

See KN4CK3R#4

@KN4CK3R
Copy link
Member Author

KN4CK3R commented Dec 29, 2022

As an aside lots of the comments on the errors have gone missing in this PR.

Not missing, I intentionaly removed them on purpose. The comments were required at the time of writing and I see no benefit in comments like

// ErrMissingComposerFile indicates a missing composer.json file
ErrMissingComposerFile = errors.New("composer.json file is missing")

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Dec 30, 2022
lunny and others added 2 commits December 30, 2022 13:10
* Add convenience functions and do some more refactoring

Signed-off-by: Andrew Thornton <art27@cantab.net>

* Apply suggestions from code review

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: KN4CK3R <admin@oldschoolhack.me>
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Dec 31, 2022
@KN4CK3R
Copy link
Member Author

KN4CK3R commented Dec 31, 2022

🚀

@KN4CK3R KN4CK3R merged commit 3fef47b into go-gitea:main Dec 31, 2022
@KN4CK3R KN4CK3R deleted the refactor-package-error branch December 31, 2022 11:50
@lunny lunny removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Dec 31, 2022
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jan 3, 2023
* upstream/main:
  Add deprecated warning for DISABLE_GRAVATAR and ENABLE_FEDERATED_AVATAR (go-gitea#22318)
  Unify hashing for avatar (go-gitea#22289)
  fix: code search title translation (go-gitea#22285)
  Update Gmail mailer configuration (go-gitea#22291)
  Fix due date rendering the wrong date in issue (go-gitea#22302)
  Fix get system setting bug when enabled redis cache (go-gitea#22295)
  Restructure `webhook` module (go-gitea#22256)
  Reminder for no more logs to console (go-gitea#22282)
  Fix bug of DisableGravatar default value (go-gitea#22296)
  Upgrade go-chi to v5.0.8 (go-gitea#22304)
  [skip ci] Updated licenses and gitignores
  Use ErrInvalidArgument in packages (go-gitea#22268)
  Changelog v1.18.0 (go-gitea#22215) (go-gitea#22269)
  Support estimated count with multiple schemas (go-gitea#22276)
  Add Gentoo to the from package providers (go-gitea#22284)
  Fix sitemap (go-gitea#22272)
  Add `sync_on_commit` option for push mirrors api (go-gitea#22271)
  Fix key signature error page (go-gitea#22229)
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. topic/packages type/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants