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

Remove dead broken code #793

Merged
merged 2 commits into from
Jan 9, 2021
Merged

Conversation

felixfontein
Copy link
Contributor

The function encodeMetadataItem in stores/ini/store.go is not used anywhere, and fails compilation (see #774).

The failure might be Go version dependent (my go version says go version go1.15.6 linux/amd64), but since the code isn't used, I guess it's best to just remove it.

@felixfontein felixfontein changed the base branch from master to develop December 29, 2020 19:35
@felixfontein
Copy link
Contributor Author

The only use of that function was removed in dc4a697, the second commit touching stores/ini/store.go.

@autrilla
Copy link
Contributor

It does seem version dependant, since it works for me on 1.13. But yeah, let's get rid of it.

@felixfontein
Copy link
Contributor Author

It seems to be more a warning with go 1.15 (golang/go#32479, golang/go#3939), but that seems to make go test fail. I don't know go well enough, maybe go test compiles with warnings as errors. In any case, simply removing the code block (since the corresponding decoding block is already long gone) really seems like the best choice :)

@felixfontein felixfontein force-pushed the remove-dead-broken-code branch from 33322a1 to 47cf265 Compare January 6, 2021 21:57
@felixfontein
Copy link
Contributor Author

@autrilla @ajvb I've rebased my two PRs to latest develop, but they still try to run tests with Travis. I think the commit which removes Travis and adds GitHub Actions needs to get pulled into master before CI works again in PRs.

@felixfontein felixfontein reopened this Jan 7, 2021
@felixfontein
Copy link
Contributor Author

@ajvb it seems that Travis is still active for this repository, which causes it to show up as a CI step which never runs and blocks completion of CI (especially since it is marked as "Required"). I guess someone with admin rights needs to disable / remove it.

@ajvb
Copy link
Contributor

ajvb commented Jan 9, 2021

@felixfontein Thanks, fixed.

@ajvb ajvb merged commit 1049773 into getsops:develop Jan 9, 2021
@felixfontein felixfontein deleted the remove-dead-broken-code branch January 9, 2021 20:06
@felixfontein
Copy link
Contributor Author

@autrilla @ajvb thanks a lot for reviewing and merging!

@tswanson-cs tswanson-cs mentioned this pull request Jan 18, 2021
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.

3 participants