Skip to content

Revert the minimal golang version requirement from 1.17 to 1.16 and add a warning in Makefile#19319

Merged
techknowlogick merged 6 commits into
go-gitea:release/v1.16from
lunny:lunny/min_go_version
Apr 5, 2022
Merged

Revert the minimal golang version requirement from 1.17 to 1.16 and add a warning in Makefile#19319
techknowlogick merged 6 commits into
go-gitea:release/v1.16from
lunny:lunny/min_go_version

Conversation

@lunny
Copy link
Copy Markdown
Member

@lunny lunny commented Apr 4, 2022

Fix #19187

@lunny lunny added the topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile label Apr 4, 2022
@lunny lunny added this to the 1.16.6 milestone Apr 4, 2022
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Apr 4, 2022
Comment thread Makefile Outdated
Comment thread Makefile Outdated
Copy link
Copy Markdown
Member

@jolheiser jolheiser left a comment

Choose a reason for hiding this comment

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

LGTM after phrasing nits.

@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 Apr 4, 2022
Co-authored-by: John Olheiser <john.olheiser@gmail.com>
Comment thread Makefile
Comment on lines 208 to +212
@if [ "$(GO_VERSION)" -lt "$(MIN_GO_VERSION)" ]; then \
echo "Gitea requires Go $(MIN_GO_VERSION_STR) or greater to build. You can get it at https://go.dev/dl/"; \
echo "Gitea requires Go $(MIN_GO_VERSION_STR) or greater to build, but $(GO_VERSION) was found. You can get an updated version at https://go.dev/dl/"; \
exit 1; \
else \
echo "WARNING: Please ensure Go $(GO_VERSION_STR) is still maintained to avoid possible security problems. You can check it at https://go.dev/dl/"; \
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Isn't it very strange for a user who uses Go1.18 see this WARNING?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Currently yes, but when Golang go to 1.2x, it will not.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think if we leave this, we can expect other issues with why this warning is appearing for users that are using the latest go version. We don't want to let them think that they are using a outdated go version when they are not.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

But we will never know it except we check Golang's website in the Makefile.

@techknowlogick
Copy link
Copy Markdown
Member

This actually won't build in 1.16 as this PR is now, due this file f9ea4ab#diff-93c289a98a6bd37986878061f982fad85e37b47dff818559e021216f9319a2a8 I will push changes to this branch to resolve before merging.

Copy link
Copy Markdown
Member

@techknowlogick techknowlogick left a comment

Choose a reason for hiding this comment

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

temporarily blocking

Comment thread modules/util/net.go Outdated
techknowlogick and others added 3 commits April 5, 2022 12:28
Co-authored-by: Gusted <williamzijl7@hotmail.com>
yay tests for catching this :)
@techknowlogick techknowlogick merged commit 0704009 into go-gitea:release/v1.16 Apr 5, 2022
Copy link
Copy Markdown
Member

@6543 6543 left a comment

Choose a reason for hiding this comment

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

lgtm

@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
@lunny lunny deleted the lunny/min_go_version branch August 24, 2023 11:20
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. topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants