Skip to content
This repository was archived by the owner on Aug 2, 2021. It is now read-only.

vendor: remove unused libraries#717

Merged
nonsense merged 1 commit into
swarm-network-rewritefrom
sync_snr_with_master
Jun 14, 2018
Merged

vendor: remove unused libraries#717
nonsense merged 1 commit into
swarm-network-rewritefrom
sync_snr_with_master

Conversation

@nonsense
Copy link
Copy Markdown
Contributor

@nonsense nonsense commented Jun 14, 2018

This PR is:

  1. Removing unused libraries from vendor.
  2. Re-enabling tests that are not Swarm-specific - hopefully they've been fixed by other teams by now.

@nonsense nonsense requested review from gbalint and janos June 14, 2018 15:00
janos
janos approved these changes Jun 14, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@janos janos left a comment

Choose a reason for hiding this comment

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

LGTM, just govendor list reports that github.com/influxdata/influxdb/client/v2 is unused too.

@holisticode
Copy link
Copy Markdown
Contributor

holisticode commented Jun 14, 2018

Shouldn't govendor list be included in the test suite somehow so there's always a report if a library becomes obsolete?

@holisticode holisticode self-requested a review June 14, 2018 17:48
holisticode
holisticode approved these changes Jun 14, 2018
View reviewed changes
Comment thread p2p/nat/natupnp_test.go
)

func TestUPNP_DDWRT(t *testing.T) {
t.Skip("broken")
Copy link
Copy Markdown
Contributor

@holisticode holisticode Jun 14, 2018

Choose a reason for hiding this comment

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

This change is not related to removing vendoring....what's the reason to re-enable this test?

Copy link
Copy Markdown
Contributor Author

@nonsense nonsense Jun 14, 2018

Choose a reason for hiding this comment

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

See comment on PR - these tests are not related to Swarm, and I don't see them failing on go-ethereum master.

Comment thread whisper/whisperv6/peer_test.go
var cntPrev int

func XTestSimulation(t *testing.T) {
func TestSimulation(t *testing.T) {
Copy link
Copy Markdown
Contributor

@holisticode holisticode Jun 14, 2018

Choose a reason for hiding this comment

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

Same here, rationale for re-enabling this test?

Copy link
Copy Markdown
Contributor Author

@nonsense nonsense Jun 14, 2018

Choose a reason for hiding this comment

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

See comment on PR - these tests are not related to Swarm, and I don't see them failing on go-ethereum master.

@nonsense nonsense merged commit a71046c into swarm-network-rewrite Jun 14, 2018
@nonsense nonsense deleted the sync_snr_with_master branch June 14, 2018 18:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@gbalint gbalint Awaiting requested review from gbalint

2 more reviewers

@janos janos janos approved these changes

@holisticode holisticode holisticode approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

ready for review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nonsense @holisticode @janos