Updates and priority changes #62

Merged
lunny merged 3 commits from jolheiser/changelog:priority into master 2022-06-22 01:27:24 +00:00
Owner

Closes #61

This PR does some general cleanup and implements pseudo-priority based on the order defined in the changelog config.

It makes use of the embed package and removes the workaround that was previously used for the default config.

It updates go.mod to go 1.18, but this shouldn't be a problem as changelog is mostly an internal tool.

It also changes a few variables to be non-exported in order to better control their scope.

Closes #61 This PR does some general cleanup and implements pseudo-priority based on the order defined in the changelog config. It makes use of the `embed` package and removes the workaround that was previously used for the default config. It updates `go.mod` to `go 1.18`, but this shouldn't be a problem as changelog is mostly an internal tool. It also changes a few variables to be non-exported in order to better control their scope.
jolheiser added 1 commit 2022-06-20 18:40:22 +00:00
Some checks failed
continuous-integration/drone/pr Build is failing
e7dfba2ca2
Updates and priority changes
Signed-off-by: jolheiser <john.olheiser@gmail.com>
6543 reviewed 2022-06-20 18:48:57 +00:00
.drone.yml Outdated
@ -15,7 +15,7 @@ steps:
pull: always
image: golang:1.17
Owner

1.18

1.18
jolheiser marked this conversation as resolved
jolheiser added 1 commit 2022-06-20 18:51:11 +00:00
Some checks failed
continuous-integration/drone/pr Build is failing
d566d8a85f
Update Go version in Drone
Signed-off-by: jolheiser <john.olheiser@gmail.com>
6543 reviewed 2022-06-20 18:51:36 +00:00
@ -51,0 +44,4 @@
}
}
if defaultGroup == "" {
defaultGroup = cfg.Groups[len(cfg.Groups)-1].Name
Owner

should we just drop if there is no default group and no group match?

should we just drop if there is no default group and no group match?
Author
Owner

I'm ambivalent, but that would technically be a breaking change. This is how the old logic worked.

I'm ambivalent, but that would technically be a breaking change. This is how the old logic worked.
Owner

something for next pull ;)

I think we should break this but agree - this is out of scope for this pull

something for next pull ;) I think we should break this but agree - this is out of scope for this pull
6543 marked this conversation as resolved
6543 reviewed 2022-06-20 19:10:26 +00:00
cmd/cmd.go Outdated
@ -1,24 +1,84 @@
// Copyright 2020 The Gitea Authors. All rights reserved.
Owner

intentional?!?

intentional?!?
Author
Owner

Ah, nope. I had deleted the file and then renamed a different one to this, but forgot a new copyright.

Ah, nope. I had deleted the file and then renamed a different one to this, but forgot a new copyright.
jolheiser marked this conversation as resolved
6543 reviewed 2022-06-20 19:14:15 +00:00
@ -27,3 +26,4 @@
$(GO) test -race -v ./...
.PHONY: lint
lint:
Owner

I think we should switch to use go install ... here

I think we should switch to use `go install ...` here
Author
Owner

Or go run, now that Go supports versioned running

Or `go run`, now that Go supports versioned running
jolheiser marked this conversation as resolved
jolheiser added 1 commit 2022-06-20 19:56:28 +00:00
All checks were successful
continuous-integration/drone/pr Build is passing
a03fb2ecad
Update golangci-lint and re-add copyright
Signed-off-by: jolheiser <john.olheiser@gmail.com>
lunny reviewed 2022-06-21 05:46:47 +00:00
@ -48,3 +38,3 @@
}
processPRs(prs, cfg.SkipRegex)
var defaultGroup string
Owner

Shouldn't these code be invoked before service.New?

Shouldn't these code be invoked before `service.New`?
Owner

I dont see it in use before ... so more a code-convention thing when to init vars?

I'm ok as it's done atm by the way...

I dont see it in use before ... so more a code-convention thing when to init vars? I'm ok as it's done atm by the way...
Author
Owner

I always init the var right before I need it. In terms of service.New, the service is only responsible for getting entries, not for grouping them. That happens further down.

I always init the var right before I need it. In terms of `service.New`, the service is only responsible for getting entries, not for grouping them. That happens further down.
lunny marked this conversation as resolved
6543 approved these changes 2022-06-21 12:08:43 +00:00
lunny approved these changes 2022-06-22 01:27:02 +00:00
lunny merged commit 57e620da5b into master 2022-06-22 01:27:24 +00:00
jolheiser added the
enhancement
label 2022-06-22 02:52:16 +00:00
jolheiser added this to the v0.3.0 milestone 2022-06-22 02:52:36 +00:00
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: gitea/changelog#62
No description provided.