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.

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

cmd: go 1.10 maintains a build cache now #1701

Merged
merged 1 commit into from
Mar 19, 2018
Merged

cmd: go 1.10 maintains a build cache now #1701

merged 1 commit into from
Mar 19, 2018

Conversation

fatih
Copy link
Owner

@fatih fatih commented Mar 2, 2018

We don't need this hack anymore to speed up building.

We don't need this hack anymore to speed up building.
@codecov-io
Copy link

Codecov Report

Merging #1701 into master will increase coverage by 0.33%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1701      +/-   ##
==========================================
+ Coverage   19.75%   20.09%   +0.33%     
==========================================
  Files          57       57              
  Lines        4738     4758      +20     
==========================================
+ Hits          936      956      +20     
  Misses       3802     3802
Flag Coverage Δ
#nvim 15.02% <ø> (-0.07%) ⬇️
#vim74 17.23% <ø> (-0.08%) ⬇️
#vim80 19.02% <ø> (+0.34%) ⬆️
Impacted Files Coverage Δ
autoload/go/cmd.vim 5.73% <ø> (ø) ⬆️
autoload/go/lint.vim 61.61% <0%> (+4.31%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eaef6f7...2447029. Read the comment docs.

@bhcleek
Copy link
Collaborator

bhcleek commented Mar 2, 2018

Do we want to remove this already? For how long should we support optimizations for past releases of go?

@fatih
Copy link
Owner Author

fatih commented Mar 3, 2018

I usually trying to keep up with latest. It helps the community to upgrade to the latest version and also makes the life easier for us (like how Apple encourages people to upgrade to the latest iOS version).

Of course not everyone can upgrade their Go version. We can check the version, like how we do it in plugin/go.vim but that will add additional latency and we need to add caching to mitigate it. I say let's merge this as this is added recently already.

@arp242
Copy link
Contributor

arp242 commented Mar 4, 2018

But it's not like go build -i does any harm, right?

Either way, I think it's fine to both merge it and keep as-is, but I do think it would be a good idea to add an entry in the BACKWARDS INCOMPATIBILITIES section of the ChangeLog.

@bhcleek
Copy link
Collaborator

bhcleek commented Mar 4, 2018

Lgtm

@fatih
Copy link
Owner Author

fatih commented Mar 19, 2018

But it's not like go build -i does any harm, right?

yeah definitely, but :GoBuild in its plan form implies that it only runs go build. But we change the semantics. Not that we didn't do it for other commands or whatsoever. Just with 1.10 this is not required anymore. Keeping things like this bloats the codebase with time because people tend to forget things in the long term. Merging this and adding an entry under BACKWARDS INCOMPATIBILITIES

@fatih fatih merged commit b580823 into master Mar 19, 2018
@fatih fatih deleted the remove-install branch March 19, 2018 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants