gitea/go-sdk
44
46
Fork 73
Code Issues 21 Pull Requests 7 Actions Releases 19 Activity

Add ignoreVersion & manuall version set option #560

Merged
6543 merged 12 commits from 6543/go-sdk:add-option-to-set/skip-version-check into master 2022-01-04 16:31:32 +00:00
Conversation 7 Commits 12 Files Changed 8 +92 -41
6543 commented 2022-01-02 16:00:01 +00:00
Owner
Copy Link

be able to skip version check if needed.

!!! Be careful, because using it incorrectly can result in infinite loops with pagination !!!

be able to skip version check if needed. !!! Be careful, because using it incorrectly can result in infinite loops with pagination !!!
6543 added 4 commits 2022-01-02 16:00:03 +00:00
add ignoreVersion & manuall version set option
All checks were successful
continuous-integration/drone/pr Build is passing
Details
e88e08c209
6543 added this to the v0.16.0 milestone 2022-01-02 16:00:13 +00:00
6543 added the
need/backport
kind/feature
 labels 2022-01-02 16:00:27 +00:00
KN4CK3R commented 2022-01-02 16:36:40 +00:00
Member
Copy Link

Should CheckServerVersionConstraint use the skip too?

Should `CheckServerVersionConstraint` use the skip too?
6543 added 1 commit 2022-01-02 17:38:16 +00:00
add test case
All checks were successful
continuous-integration/drone/pr Build is passing
Details
ab509fe02f
6543 commented 2022-01-02 17:57:38 +00:00
Author
Owner
Copy Link

@KN4CK3R this is an exported func and should not used internaly ...

to the question: why would you externaly first set ignoreVersion and check for it avterwards ... sounds like a bug to me

and if you set a version manualy ... added test case and fixed it ;) ... you will use that one with this CheckServerVersionConstraint() too

@KN4CK3R this is an exported func and should not used internaly ... to the question: why would you externaly first set ignoreVersion and check for it avterwards ... sounds like a bug to me and if you set a version manualy ... added test case and fixed it ;) ... you will use that one with this **CheckServerVersionConstraint()** too
6543 added 2 commits 2022-01-02 17:57:54 +00:00
fix
Some checks failed
continuous-integration/drone/pr Build is failing
Details
bb4795a41b
6543 added 1 commit 2022-01-02 17:58:55 +00:00
fix test
All checks were successful
continuous-integration/drone/pr Build is passing
Details
ccef7964f4
zeripath commented 2022-01-02 18:26:56 +00:00
Owner
Copy Link

Gitea calls CheckServerVersionConstraint in the migrations downloaders. I guess the question is if we'd want to allow ourselves to set ignore there?

Gitea calls CheckServerVersionConstraint in the migrations downloaders. I guess the question is if we'd want to allow ourselves to set ignore there?
6543 commented 2022-01-02 18:29:07 +00:00
Author
Owner
Copy Link

@zeripath well we can set a valid version if we want to ...

@zeripath well we can set a valid version if we want to ...
6543 commented 2022-01-02 18:29:47 +00:00
Author
Owner
Copy Link

... so for tests its even more interesting to "pretend" to have a specific kind of version to test against

... so for tests its even more interesting to "pretend" to have a specific kind of version to test against
6543 added 1 commit 2022-01-02 18:35:21 +00:00
Merge branch 'master' into add-option-to-set/skip-version-check
All checks were successful
continuous-integration/drone/pr Build is passing
Details
2a0feef45d
6543 reviewed 2022-01-02 18:36:36 +00:00
gitea/client.go Outdated
@ -50,2 +52,4 @@
}
// ClientOptions are functions used to init a new client
type ClientOptions func(*Client) error
6543 commented 2022-01-02 18:36:36 +00:00
Author
Owner
Copy Link

just a thought ClientOptions or ClientOption ?

just a thought `ClientOptions` or `ClientOption` ?
noerw commented 2022-01-02 19:11:07 +00:00
Member
Copy Link

I vote ClientOption

I vote `ClientOption`
6543 marked this conversation as resolved
6543 added 1 commit 2022-01-02 19:14:01 +00:00
ClientOptions -> ClientOption
All checks were successful
continuous-integration/drone/pr Build is passing
Details
4034e9458f
6543 requested review from noerw 2022-01-02 19:17:42 +00:00
zeripath approved these changes 2022-01-02 23:32:03 +00:00
lunny commented 2022-01-03 01:44:26 +00:00
Owner
Copy Link

Why not just name the function IgnoreVersionCheck? Do we really need to allow change the version number? Assume someone will change the version from 1.11.0 to 1.16.0?

Why not just name the function `IgnoreVersionCheck`? Do we really need to allow change the version number? Assume someone will change the version from `1.11.0` to `1.16.0`?
wxiaoguang reviewed 2022-01-03 03:12:40 +00:00
gitea/version.go Outdated
@ -42,0 +42,4 @@
// SetGiteaVersion sets the Gitea version manually instead of querying it.
// use "0.0.0" to skip all version checks
func SetGiteaVersion(v string) ClientOption {
if v == "0.0.0" {
wxiaoguang commented 2022-01-03 03:12:40 +00:00
Member
Copy Link

"0.0.0" seems a strange magic string.

"0.0.0" seems a strange magic string.
6543 commented 2022-01-03 15:30:17 +00:00
Author
Owner
Copy Link

well other option would be "" or make SetGiteaVersion(v *string) with v == nil check

well other option would be "" or make SetGiteaVersion(v *string) with v == nil check
6543 marked this conversation as resolved
noerw reviewed 2022-01-03 10:45:48 +00:00
gitea/version.go Outdated
@ -39,6 +39,24 @@ func (c *Client) CheckServerVersionConstraint(constraint string) error {
return nil
}
// SetGiteaVersion sets the Gitea version manually instead of querying it.
noerw commented 2022-01-03 10:45:48 +00:00
Member
Copy Link
-// SetGiteaVersion sets the Gitea version manually instead of querying it.
+// SetGiteaVersion configures the Client to assume the given version of the
+// Gitea server, instead of querying the server for it when initializing.

Also agree that "0.0.0" is a bit strange, maybe use empty string instead?

```diff -// SetGiteaVersion sets the Gitea version manually instead of querying it. +// SetGiteaVersion configures the Client to assume the given version of the +// Gitea server, instead of querying the server for it when initializing. ``` Also agree that `"0.0.0"` is a bit strange, maybe use empty string instead?
👍 1
6543 marked this conversation as resolved
noerw reviewed 2022-01-03 10:49:04 +00:00
gitea/version.go
@ -50,6 +68,9 @@ var (
// checkServerVersionGreaterThanOrEqual is internally used to speed up things and ignore issues with prerelease
noerw commented 2022-01-03 10:49:04 +00:00
Member
Copy Link
-// checkServerVersionGreaterThanOrEqual is internally used to speed up things and ignore issues with prerelease
+// checkServerVersionGreaterThanOrEqual is the canonical way in the SDK to check for versions for API compatibility reasons
```diff -// checkServerVersionGreaterThanOrEqual is internally used to speed up things and ignore issues with prerelease +// checkServerVersionGreaterThanOrEqual is the canonical way in the SDK to check for versions for API compatibility reasons ```
6543 marked this conversation as resolved
noerw reviewed 2022-01-03 10:50:37 +00:00
gitea/version_test.go
@ -18,6 +18,15 @@ func TestVersion(t *testing.T) {
assert.NoError(t, err)
assert.True(t, true, rawVersion != "")
noerw commented 2022-01-03 10:50:37 +00:00
Member
Copy Link

Somewhat unrelated, but prompted by the docstring above:
Maybe we need tests for typical version strings returned by gitea, including pre-release like 1.16.0+dev-661-geb69c7ec8

Somewhat unrelated, but prompted by the docstring above: Maybe we need tests for typical version strings returned by gitea, including pre-release like `1.16.0+dev-661-geb69c7ec8`
6543 commented 2022-01-03 15:33:12 +00:00
Author
Owner
Copy Link

I dont think we should do that -> it should be ensured by our version lib

(github.com/hashicorp/go-version) - but I would sugest to migrate to my fork as it accepts pulls ;) (at gitea we already use it - just as redirect for now but the needed patches never got accepted)

I dont think we should do that -> **it should be ensured by our version lib** (github.com/hashicorp/go-version) - but I would sugest to migrate to my fork as it accepts pulls ;) (at gitea we already use it - just as redirect for now but the needed patches never got accepted)
6543 commented 2022-01-03 15:29:22 +00:00
Author
Owner
Copy Link

Why not just name the function IgnoreVersionCheck ...

I'm thinking of gitea forks like https://allspice.io have - they could differ in versioning schema ...

> Why not just name the function IgnoreVersionCheck ... I'm thinking of gitea forks like https://allspice.io have - they could differ in versioning schema ...
6543 added 1 commit 2022-01-03 15:42:04 +00:00
wordings & "" for ignroe
All checks were successful
continuous-integration/drone/pr Build is passing
Details
880d7d9217
6543 added 1 commit 2022-01-03 15:43:31 +00:00
more wordings
All checks were successful
continuous-integration/drone/pr Build is passing
Details
a63baf5b3c
noerw approved these changes 2022-01-04 13:30:12 +00:00
6543 merged commit 635de1b821 into master 2022-01-04 16:31:32 +00:00
6543 deleted branch add-option-to-set/skip-version-check 2022-01-04 16:31:32 +00:00
6543 added
has/backport
skip-changelog
 and removed
need/backport
 labels 2022-01-04 16:37:03 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
zeripath
noerw
Labels
Clear labels   
has/backport

   
has/pull

   
in progress

   
invalid

   
kind/breaking

   
kind/bug

   
kind/build

   
kind/deployment

   
kind/docs

   
kind/enhancement

   
kind/feature

   
kind/lint

   
kind/proposal

   
kind/question

   
kind/refactor

   
kind/security

   
kind/testing

   
kind/translation

   
kind/ui

   
need/backport

   
priority/critical

   
priority/low

   
priority/maybe

   
priority/medium

   
reviewed/duplicate

   
reviewed/invalid

   
reviewed/wontfix

   
skip-changelog

   
status/blocked

   
status/needs-feedback

   
status/needs-reviews

   
status/wip

   
upstream/gitea
No Label
has/backport
has/pull
in progress
invalid
kind/breaking
kind/bug
kind/build
kind/deployment
kind/docs
kind/enhancement
kind/feature
kind/lint
kind/proposal
kind/question
kind/refactor
kind/security
kind/testing
kind/translation
kind/ui
need/backport
priority/critical
priority/low
priority/maybe
priority/medium
reviewed/duplicate
reviewed/invalid
reviewed/wontfix
skip-changelog
status/blocked
status/needs-feedback
status/needs-reviews
status/wip
upstream/gitea
Milestone
Clear milestone
No items
No Milestone
v0.16.0
Assignees
Clear assignees
No Assignees
6 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/go-sdk#560
No description provided.
Delete Branch "6543/go-sdk:add-option-to-set/skip-version-check"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?