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

refactor: remove workaround after the golang 1.7 release. #1349

Merged
merged 5 commits into from
Mar 29, 2017
Merged

refactor: remove workaround after the golang 1.7 release. #1349

merged 5 commits into from
Mar 29, 2017

Conversation

appleboy
Copy link
Member

@appleboy appleboy commented Mar 21, 2017 •
edited
Loading

This workaround has been fixed on Go 1.7.

@lunny lunny added this to the 1.2.0 milestone Mar 21, 2017
@lunny lunny added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Mar 21, 2017
@bkcsoft
Copy link
Member

bkcsoft commented Mar 21, 2017

We still support 1.6 though AFAIK

@tboerger tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 21, 2017
@bkcsoft bkcsoft added the issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail label Mar 21, 2017
@appleboy
Copy link
Member Author

@bkcsoft I think we can recommend user using go 1.7 version to build binary and update README. This main purpose for this repo is offering the binary, not package.

@strk
Copy link
Member

strk commented Mar 22, 2017

I think one of the purposes of a free software project is making users able to exercise their freedom of building/modifying the software. The least barriers we put to build, the better. Latest stable Ubuntu distribution (not Debian) comes with Go 1.6, I would not drop support of that.

My rule of thumb to be friendly with free software developers is: reduce deps to those available on latest Debian Stable. Does anyone know what that ships ?

@strk
Copy link
Member

strk commented Mar 22, 2017

So Debian 8 ships 1.6.1, Ubuntu LTS ships 1.6.2 - I am using 1.6.2 for everyday development, so please don't make my contributions harder

@lunny
Copy link
Member

lunny commented Mar 22, 2017

We should add a tag to ask go build version.

@strk
Copy link
Member

strk commented Mar 22, 2017

If you handle to split the file so to have go-version specific versions, chances are build tags on that are implicit

@appleboy
Copy link
Member Author

@lunny @bkcsoft Done. Please review again.

@lunny
Copy link
Member

lunny commented Mar 24, 2017

LGTM

@tboerger tboerger added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 24, 2017
@appleboy appleboy removed the issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail label Mar 24, 2017
@pgaskin
Copy link
Contributor

pgaskin commented Mar 29, 2017

Seems fine. LGTM

@tboerger tboerger removed the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Mar 29, 2017
@appleboy
Copy link
Member Author

@strk Any concerns about this? We use os.RemoveAll function in windows after go1.7 now and backward compatibility.

@lunny lunny merged commit 08f7fde into go-gitea:master Mar 29, 2017
@strk
Copy link
Member

strk commented Mar 29, 2017

Is the go1.7 tag implicit? LGTM anyway

@appleboy appleboy mentioned this pull request Apr 18, 2017
Merged
@appleboy appleboy deleted the refactor3 branch April 18, 2017 07:16
@go-gitea go-gitea locked and limited conversation to collaborators Nov 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
type/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Milestone
1.2.0
Development

Successfully merging this pull request may close these issues.
6 participants
@appleboy @bkcsoft @strk @lunny @pgaskin @tboerger