-
Notifications
You must be signed in to change notification settings - Fork 110
Conversation
@@ -21,19 +21,6 @@ jobs: | |||
script: | |||
- go run build/ci.go lint | |||
|
|||
# Go 1.12.x is needed because of the Ubuntu PPA builds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Go 1.12 is not needed for PPA, but it is a supported Go version. Last two releases are. Maybe we should keep it for this reason and change the comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to support go1.12?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought that we supported current and previous go version. I could be wrong. If we are supporting only the current, then the job is not needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously we were supporting 1.11 and 1.12 mainly because the Ubuntu PPA needs 1.11.
IMO we can remove 1.12 because none of our build systems need it. Our binaries are published using 1.13.
I'm also in favor of getting rid of 1.11 and the Ubuntu PPA builds, but that's a topic for the roundtable :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe our codebase does not need to ensure support for previous go version for people that cannot upgrade right away, as it is easy to have two version one beside another. In that case, removing this job is ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i also thought it was deliberate check for backward compatibility one version back. I would leave it. is it really making CI faster given parallelism?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Backward compatibility is not needed IMO. Our binaries are released with go1.13, so why should we care about supporting go1.12?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that this is an open source project, someone may still use older but still supported Go version to build or use some of the packages from swarm. Maybe we still did not reach that level of project maturity or popularity, so ensuring that project works only with the latest release is just fine, and that we can think about this later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a comment on the CI from my side - we have 5 concurrent jobs in total for the ethersphere
organisation, so if half of them are used on jobs we don't need, CI is essentially twice slower, specially when you have to wait for free executors - yesterday we waited for one-two hours to get helm charts released, due to many PRs being submitted at the same time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we still did not reach that level of project maturity or popularity, so ensuring that project works only with the latest release is just fine, and that we can think about this later.
- exactly my opinion on this PR.
Thanks for the reviews and valid opinions. I'll merge it for now. We can still add it back If we ever have a case where someone need this to be compatible with another go version. |
* 'master' of github.com:ethersphere/swarm: (32 commits) network/stream: refactor cursors tests (ethersphere#1786) network: Add capabilities if peer from store does not have it (ethersphere#1791) Swap logger (ethersphere#1754) network: Add capability filtered depth calculation (ethersphere#1787) travis: remove go1.12 job (ethersphere#1784) cmd/swarm: correct bzznetworkid flag description (ethersphere#1761) network, pss: Capability in pss (ethersphere#1764) network/stream: handle nil peer in TestNodesExchangeCorrectBinIndexes (ethersphere#1779) protocols, retrieval: swap-enabled messages implement Price (ethersphere#1771) cmd/swarm-smoke: fix waitToPushSynced connection closing (ethersphere#1781) cmd/swarm: simplify testCluster.StartNewNodes (ethersphere#1777) build: increase golangci-lint deadline (ethersphere#1778) docker: ignore build/bin when copying files (ethersphere#1780) swap: fix and rename Peer.getLastSentCumulativePayout (ethersphere#1769) network/stream: more resilient TestNodesCorrectBinsDynamic (ethersphere#1776) network: Add Capabilities to Kademlia database (ethersphere#1713) network: add own address to KademliaInfo (ethersphere#1775) pss: Refactor. Step 2. Refactor forward cache (ethersphere#1742) all: configurable payment/disconnect thresholds (ethersphere#1729) network/stream/v2: more resilient TestNodesExchangeCorrectBinIndexes (ethersphere#1760) ...
This is not needed. The Ubuntu PPA is using go1.11.