Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implement N-1 support policy for Go versions #4653

Closed
2 of 3 tasks
yurishkuro opened this issue Aug 12, 2023 · 5 comments · Fixed by #4655
Closed
2 of 3 tasks

Implement N-1 support policy for Go versions #4653

yurishkuro opened this issue Aug 12, 2023 · 5 comments · Fixed by #4655
Labels
enhancement good first issue Good for beginners help wanted Features that maintainers are willing to accept but do not have cycles to implement

Comments

@yurishkuro
Copy link
Member

yurishkuro commented Aug 12, 2023 •
edited

Follow up to #4329.

Since Go 1.21 is out, this is an opportunity to test this new process.
@yurishkuro yurishkuro added help wanted Features that maintainers are willing to accept but do not have cycles to implement good first issue Good for beginners labels Aug 12, 2023
@yurishkuro yurishkuro mentioned this issue Aug 12, 2023
Closed
@shubmjagtap
Copy link

Hi @yurishkuro ,

in task 1 : "verifies that all our build scripts are using the same Go version N"

1.how do i find those build scripts from such large code base ?
2.Are these {build scripts} present in "script" folder as ".sh" files?

@wasup-yash
Copy link

wasup-yash commented Aug 13, 2023 •
edited

hey @yurishkuro what is the estimated/expected time limit if any for these issues to get resolved/implemented?

@Mayhul-Jindal
Copy link

Hey @yurishkuro as I was trying to setup the project I encountered this issue
image
I have already tried removing the go.sum and then regenerated it using go mod tidy but still if gave me the same error. I am new to contributing and stuff, can you suggest me what should I do next ?

@Mayhul-Jindal
Copy link

Hey @yurishkuro as I was trying to setup the project I encountered this issue image I have already tried removing the go.sum and then regenerated it using go mod tidy but still if gave me the same error. I am new to contributing and stuff, can you suggest me what should I do next ?

Continuing this, I faced issue in running make lint because github.com/golangci/golangci-lint/cmd/golangci-lint@v1.52.1 was not able to install correctly. When I installed its updated version go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.54.1 everything seems to work fine now. Should the version be changed to 1.54.1 ?
image

@perhapsmaple perhapsmaple mentioned this issue Aug 13, 2023
Merged
4 tasks
@albertteoh
Copy link
Contributor

Should the version be changed to 1.54.1 ?

If we upgrade golangci-lint, we'll be subject to the problem described in: golangci/golangci-lint#3906, where a large number of depguard lint errors appear.

We'll need to understand why those linting errors are appearing and what actions need to be taken to address them, or if it's better to stick with the current version until that problem is ironed out in a later version.

FWIW, I didn't have any issues installing 1.52.1.

yurishkuro added a commit that referenced this issue Sep 4, 2023
@perhapsmaple @yurishkuro
Implement N-1 support policy for Go versions (#4655)
fb8eb7d 
## Which problem is this PR solving?
Resolves  #4653

## Description of the changes
- Current files tracked are go.mod, all yml files in ./github/workflows/
and ./docker/Makefile. If there are any more build scripts to track, I
would be happy to update the PR.
- Added scripts/check-go-version.sh to verify that all our build scripts
are using the same Go version N, but go.mod is N-1 and fail if not, with
an error message and the source files along with their respective
versions.
- Added scripts/update-go-version.sh to update those scripts to latest
version, to make these upgrades easier in the future. Also bumps go.mod
to version N-1.
- Updated README explaining the support policy.

## How was this change tested?
- I have tested the changes locally by modifying go versions

![Screenshot_20230813_120937](https://github.com/jaegertracing/jaeger/assets/140232061/bb4d9e18-22b5-4fcc-9c2b-d374bec0bc4a)

![Screenshot_20230813_121021](https://github.com/jaegertracing/jaeger/assets/140232061/602074bc-7285-48ee-97bf-ea5f0e21a586)

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [ ] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

---------

Signed-off-by: Harish Shan <140232061+perhapsmaple@users.noreply.github.com>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Co-authored-by: Yuri Shkuro <github@ysh.us>
@yurishkuro yurishkuro mentioned this issue Sep 4, 2023
Merged
yurishkuro added a commit that referenced this issue Sep 5, 2023
@yurishkuro
Move build to Go 1.21 (#4724)
f8c1523 
## Which problem is this PR solving?
- Move to next Go version per maintenance policy (see #4653)

## Description of the changes
Ran the script 
```shell
$ ./scripts/check-go-version.sh -u
go.mod                                             Go version: 1.20
docker/Makefile                                    Go version: 1.20 *** => 1.21 ***
.github/workflows/fossa.yml                        Go version: 1.20 *** => 1.21 ***
.github/workflows/ci-release.yml                   Go version: 1.20 *** => 1.21 ***
.github/workflows/ci-unit-tests.yml                Go version: 1.20 *** => 1.21 ***
.github/workflows/ci-docker-build.yml              Go version: 1.20 *** => 1.21 ***
.github/workflows/ci-build-binaries.yml            Go version: 1.20 *** => 1.21 ***
.github/workflows/ci-crossdock.yml                 Go version: 1.20 *** => 1.21 ***
.github/workflows/ci-elasticsearch.yml             Go version: 1.20 *** => 1.21 ***
.github/workflows/ci-grpc-badger.yml               Go version: 1.20 *** => 1.21 ***
.github/workflows/ci-hotrod.yml                    Go version: 1.20 *** => 1.21 ***
.github/workflows/ci-protogen-tests.yml            Go version: 1.20 *** => 1.21 ***
.github/workflows/ci-all-in-one-build.yml          Go version: 1.20 *** => 1.21 ***
.github/workflows/ci-cassandra.yml                 Go version: 1.20 *** => 1.21 ***
.github/workflows/ci-opensearch.yml                Go version: 1.20 *** => 1.21 ***
.github/workflows/ci-kafka.yml                     Go version: 1.20 *** => 1.21 ***
.golangci.yml                                      Go version: 1.20
15 file(s) updated.
```

Signed-off-by: Yuri Shkuro <github@ysh.us>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement good first issue Good for beginners help wanted Features that maintainers are willing to accept but do not have cycles to implement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Implement N-1 support policy for Go versions perhapsmaple/jaeger
5 participants
@yurishkuro @albertteoh @Mayhul-Jindal @wasup-yash @shubmjagtap