[Add] VersionCheck #215

Merged
lafriks merged 12 commits from 6543/go-sdk:version-check into master 2 years ago
6543 commented 2 years ago
Collaborator

add function to check gitea versions

use versionCheck for new introduced APIs of gitea_v1.11.0

close #197

add function to check gitea versions use versionCheck for new introduced APIs of gitea_v1.11.0 close #197
6543 added the
kind/feature
kind/proposal
labels 2 years ago
6543 changed title from version-check to [Add] VersionCheck 2 years ago
zeripath reviewed 2 years ago
Dismissed
gitea/client.go Outdated
// Version return the library version
func Version() string {
return "0.12.3"
return "0.11.0"
Owner

I don't think we can really reduce the version of the library - I see what you're trying to do though - maybe we should just make it 1.11.0?

I don't think we can really reduce the version of the library - I see what you're trying to do though - maybe we should just make it 1.11.0?
6543 commented 2 years ago
Poster
Collaborator

I dont like to go out of 0.x jet :https://medium.com/@aman.sardana/go-modules-versioning-dependency-management-d5f96b490774

"...version 0 is an exception for backward compatibility rule of golang as the version updated before version 1 can be backward incompatible. It is best for the libraries that are just starting. ..."

at this point we still can remove stuff ...
if we are sure that we stay in this structure we can go on to 1.x

I dont like to go out of 0.x jet :https://medium.com/@aman.sardana/go-modules-versioning-dependency-management-d5f96b490774 "...version 0 is an exception for backward compatibility rule of golang as the version updated before version 1 can be backward incompatible. It is best for the libraries that are just starting. ..." at this point we still can remove stuff ... if we are sure that we stay in this structure we can go on to 1.x
Owner

I'd probably argue that we don't need to vendor any more - but I guess it depends on our lowest supported version of golang.

I'd probably argue that we don't need to vendor any more - but I guess it depends on our lowest supported version of golang.
Poster
Collaborator

@zeripath

I dont like to go out of 0.x jet :https://medium.com/@aman.sardana/go-modules-versioning-dependency-management-d5f96b490774

“…version 0 is an exception for backward compatibility rule of golang as the version updated before version 1 can be backward incompatible. It is best for the libraries that are just starting. …”

at this point we still can remove stuff … if we are sure that we stay in this structure we can go on to 1.x

@zeripath I dont like to go out of 0.x jet :https://medium.com/@aman.sardana/go-modules-versioning-dependency-management-d5f96b490774 “…version 0 is an exception for backward compatibility rule of golang as the version updated before version 1 can be backward incompatible. It is best for the libraries that are just starting. …” at this point we still can remove stuff … if we are sure that we stay in this structure we can go on to 1.x
Owner

@6543 I will agree with @zeripath that we don't need vendor the dependencies on this library project.

@6543 I will agree with @zeripath that we don't need vendor the dependencies on this library project.
6543 changed title from [Add] VersionCheck to [WIP] [Add] VersionCheck 2 years ago
lafriks approved these changes 2 years ago
Dismissed
lafriks left a comment
Collaborator

For performance reasons it would be better to request server version only once in NewClient

For performance reasons it would be better to request server version only once in `NewClient`
gitea/version.go Outdated
"github.com/hashicorp/go-version"
)
var serverVersion *version.Version
Collaborator

This should be moved into Client struct as field serverVersion string

This should be moved into `Client` struct as field `serverVersion string`
gitea/version.go Outdated
var serverVersion *version.Version
// ServerVersion returns the version of the server
func (c *Client) ServerVersion() (string, error) {
Collaborator
func (c *Client) ServerVersion() (string, error) {
	if len(c.serverVersion) != 0 {
		return c.serverVersion, nil
	}
	if err := c.ReloadServerVersion(); err != nil {
		return "", err
	}
	return c.serverVersion, nil
}

func (c *Client) ReloadServerVersion() error {
	// Reset cached server version string
	c.serverVersion = ""
	var v = struct {
		Version string `json:"version"`
	}{}
	if err := c.getParsedResponse("GET", "/version", nil, nil, &v) != nil {
		return err
	}
	c.serverVersion = v.Version
	return nil
}
```go func (c *Client) ServerVersion() (string, error) { if len(c.serverVersion) != 0 { return c.serverVersion, nil } if err := c.ReloadServerVersion(); err != nil { return "", err } return c.serverVersion, nil } func (c *Client) ReloadServerVersion() error { // Reset cached server version string c.serverVersion = "" var v = struct { Version string `json:"version"` }{} if err := c.getParsedResponse("GET", "/version", nil, nil, &v) != nil { return err } c.serverVersion = v.Version return nil } ```
lafriks reviewed 2 years ago
Dismissed
gitea/client.go Outdated
accessToken: token,
client: &http.Client{},
}
if raw, err := c.ServerVersion(); err == nil {
Collaborator

Not needed here, load server version only when needed

Not needed here, load server version only when needed
lafriks requested changes 2 years ago
Dismissed
gitea/client.go Outdated
// Client represents a Gitea API client.
type Client struct {
url string
accessToken string
Collaborator
	serverVersion string
```go serverVersion string ```
6543 changed title from [WIP] [Add] VersionCheck to [Add] VersionCheck 2 years ago
Poster
Collaborator

@lafriks can you review again?

@lafriks can you review again?
6543 added this to the v0.11.0 milestone 2 years ago
lunny requested changes 2 years ago
Dismissed
gitea/version.go Outdated
// CheckServerVersionConstraint validates that the login's server satisfies a
// given version constraint such as ">= 1.11.0+dev"
func (c *Client) CheckServerVersionConstraint(constraint string) error {
lunny commented 2 years ago
Owner

I think you should have a lock since there maybe multiple goroutines check the verions.

I think you should have a lock since there maybe multiple goroutines check the verions.
Poster
Collaborator

@lunny hope 9e722bb47272557c4aceb361dee8a4247743f8f1 is wat you want ...

@lunny hope 9e722bb47272557c4aceb361dee8a4247743f8f1 is wat you want ...
Owner

Please resolve conflicts.

Please resolve conflicts.
Poster
Collaborator

done

done
Poster
Collaborator

@lafriks are you ok as it is now?

@lafriks are you ok as it is now?
lafriks reviewed 2 years ago
Dismissed
gitea/version.go Outdated
// given version constraint such as ">= 1.11.0+dev"
func (c *Client) CheckServerVersionConstraint(constraint string) (err error) {
setV := func() {
raw, _ := c.ServerVersion()
Collaborator

What if there is error?

What if there is error?
6543 commented 2 years ago
Poster
Collaborator

raw = "" -> CheckServerVersionConstraint will always end with return fmt.Errorf("gitea se...

raw = "" -> CheckServerVersionConstraint will always end with `return fmt.Errorf("gitea se...`
Collaborator

You can use sync.RWMutex instead to be able to control behavior with errors better

You can use sync.RWMutex instead to be able to control behavior with errors better
Poster
Collaborator

@lafriks lunny like to prevent a race condition when somebody is doing this:

go client.GetIssueCommentReactions()
client.GetIssueCommentReactions()

and so I used sync.Once ...

@lafriks lunny like to prevent a race condition when somebody is doing this: ```go go client.GetIssueCommentReactions() client.GetIssueCommentReactions() ``` and so I used sync.Once ...
Poster
Collaborator

@lafriks sync.RWMutex is nice!

@lafriks **sync.RWMutex** is nice!
Poster
Collaborator

I name it neutral so it client.propertyLock can be used for altering any type of property

EDIT: outdated

I name it neutral so it **client.propertyLock** can be used for altering any type of property EDIT: outdated
lafriks approved these changes 2 years ago
Dismissed
lafriks approved these changes 2 years ago
Dismissed
6543 removed the
kind/proposal
label 2 years ago
lunny reviewed 2 years ago
Dismissed
gitea/version.go Outdated
lunny commented 2 years ago
Owner

Why not cache the version here?

Why not cache the version here?
Owner

I think we should also enable -race flag on go test.
And a goroutine test is better.

I think we should also enable `-race` flag on `go test`. And a goroutine test is better.
Poster
Collaborator

@lunny enabled race

@lunny enabled race
lunny approved these changes 2 years ago
Dismissed
Poster
Collaborator

Ready to merge :)

Ready to merge :)
lafriks approved these changes 2 years ago
Dismissed
lafriks referenced this issue from a commit 2 years ago
[Add] VersionCheck (#215) enable race test make go-vet happy code format secound Func RWMutex fix prevent race condition add TEST cleanup make go-version work without vendoring dont change library version use Server Version Check on NewClient save serverVersion Makefile: export test env var (#234) exporte test var to env on test target Co-authored-by: 6543 <6543@obermui.de> Reviewed-on: https://gitea.com/gitea/go-sdk/pulls/234 Reviewed-by: techknowlogick <techknowlogick@gitea.io> Reviewed-by: John Olheiser <john.olheiser@gmail.com> use golangci-lint and revive for linting (match main repo) (#220) Co-authored-by: 6543 <6543@noreply.gitea.io> Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com> Co-authored-by: John Olheiser <john.olheiser@gmail.com> Reviewed-on: https://gitea.com/gitea/go-sdk/pulls/220 Reviewed-by: 6543 <6543@noreply.gitea.io> Reviewed-by: John Olheiser <john.olheiser@gmail.com> [Makefile] Add "test-instance"; Add "help" (#231) PASSWORD_COMPLEXITY = off fix test Makefile: add "test-instance" (start a gitea instance for test) and add a help menue Fix ListIssue Functions (now respect ListIssueOption's) (#225) fix test add Test add more test cases and fix nice log add Issue Tests impruve more Repo Tests and mv createTestRepo introduce "createTestRepo" a standad func to create a repo for testing add workaround * Update Dates * Fix ListIssueOption Fix ListRepoPullRequests (#219) add ToDo notice add ListRepoPullRequests TEST remove useless drone config emtrys fmt ping CI add new Options from PR #217 use query params Add some PR list options (#217) Empty Commit Add enums Add some PR list options Add test framework (#227) [Extend] StopWatch struct & functions (#211) add StopWatch struct & functions [Add] reaction struct and functions (#213) add struct and functions Co-authored-by: 6543 <6543@obermui.de> Reviewed-by: Andrew Thornton <art27@cantab.net> Reviewed-by: techknowlogick <techknowlogick@gitea.io> [Add] issue Un-/Subscription function (#214) fix lint add issue subscription function Co-authored-by: 6543 <6543@obermui.de> Reviewed-by: Andrew Thornton <art27@cantab.net> Reviewed-by: techknowlogick <techknowlogick@gitea.io> [Add] GetBlob (#212) fix header from PR 206 add GetBlob Co-authored-by: 6543 <6543@obermui.de> Reviewed-by: Lunny Xiao <xiaolunwen@gmail.com> Reviewed-by: Andrew Thornton <art27@cantab.net> Co-authored-by: 6543 <6543@obermui.de> Reviewed-by: Andrew Thornton <art27@cantab.net> Reviewed-by: techknowlogick <techknowlogick@gitea.io> Co-authored-by: jolheiser <john.olheiser@gmail.com> Co-authored-by: techknowlogick <techknowlogick@gitea.io> Co-authored-by: 6543 <6543@noreply.gitea.io> Reviewed-by: techknowlogick <techknowlogick@gitea.io> Reviewed-by: 6543 <6543@noreply.gitea.io> Add test framework (#227) Co-authored-by: 6543 <6543@obermui.de> Co-authored-by: John Olheiser <john.olheiser@gmail.com> Co-authored-by: techknowlogick <techknowlogick@gitea.io> Reviewed-by: techknowlogick <techknowlogick@gitea.io> Reviewed-by: Lunny Xiao <xiaolunwen@gmail.com> Add some PR list options (#217) Empty Commit Add enums Add some PR list options Add test framework (#227) [Extend] StopWatch struct & functions (#211) add StopWatch struct & functions [Add] reaction struct and functions (#213) add struct and functions Co-authored-by: 6543 <6543@obermui.de> Reviewed-by: Andrew Thornton <art27@cantab.net> Reviewed-by: techknowlogick <techknowlogick@gitea.io> [Add] issue Un-/Subscription function (#214) fix lint add issue subscription function Co-authored-by: 6543 <6543@obermui.de> Reviewed-by: Andrew Thornton <art27@cantab.net> Reviewed-by: techknowlogick <techknowlogick@gitea.io> [Add] GetBlob (#212) fix header from PR 206 add GetBlob Co-authored-by: 6543 <6543@obermui.de> Reviewed-by: Lunny Xiao <xiaolunwen@gmail.com> Reviewed-by: Andrew Thornton <art27@cantab.net> Co-authored-by: 6543 <6543@obermui.de> Reviewed-by: Andrew Thornton <art27@cantab.net> Reviewed-by: techknowlogick <techknowlogick@gitea.io> Co-authored-by: jolheiser <john.olheiser@gmail.com> Co-authored-by: techknowlogick <techknowlogick@gitea.io> Co-authored-by: 6543 <6543@noreply.gitea.io> Reviewed-by: techknowlogick <techknowlogick@gitea.io> Reviewed-by: 6543 <6543@noreply.gitea.io> Add test framework (#227) Co-authored-by: 6543 <6543@obermui.de> Co-authored-by: John Olheiser <john.olheiser@gmail.com> Co-authored-by: techknowlogick <techknowlogick@gitea.io> Reviewed-on: https://gitea.com/gitea/go-sdk/pulls/225 Reviewed-by: Andrew Thornton <art27@cantab.net> Reviewed-by: Lunny Xiao <xiaolunwen@gmail.com> Co-authored-by: 6543 <6543@obermui.de> Reviewed-on: https://gitea.com/gitea/go-sdk/pulls/231 Reviewed-by: Lunny Xiao <xiaolunwen@gmail.com> Reviewed-by: John Olheiser <john.olheiser@gmail.com> Fix ListIssue Functions (now respect ListIssueOption's) (#225) fix test add Test add more test cases and fix nice log add Issue Tests impruve more Repo Tests and mv createTestRepo introduce "createTestRepo" a standad func to create a repo for testing add workaround * Update Dates * Fix ListIssu... Co-authored-by: 6543 <6543@obermui.de> Co-authored-by: Lauris BH <lauris@nix.lv> Co-authored-by: techknowlogick <techknowlogick@gitea.io> Co-authored-by: John Olheiser <john.olheiser@gmail.com> Reviewed-on: https://gitea.com/gitea/go-sdk/pulls/215 Reviewed-by: Lunny Xiao <xiaolunwen@gmail.com> Reviewed-by: lafriks <lafriks@noreply.gitea.io>
lafriks closed this pull request 2 years ago
lafriks deleted branch version-check 2 years ago
continuous-integration/drone/pr Build is passing
The pull request has been merged as c02398aaf3.
Sign in to join this conversation.
Loading…
There is no content yet.