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

Maintenance #341

Closed
wants to merge 17 commits into from
Closed

Maintenance #341

wants to merge 17 commits into from

Conversation

klingtnet
Copy link
Contributor

This pull request does a bit of project maintenance:

CI Improvements

  • Travis CI is replaced by Github Actions
  • Go runtimes are updated, tests run against Go 1.12-Go 1.14
  • Tests are now run against Windows and Linux
  • The code is now linted

Project improvements

  • hound is now a Go modules project
  • go-bindata dependency is updated and replaced by a maintained fork
  • all linter errors are now fixed (removal of unused code, unchecked errors¹)

¹ unmmap fails in the write unit tests if the error is propagated. Not sure if this is a problem outside the test environment. Fixing this is outside the scope of this PR.

Andreas Linz added 17 commits February 26, 2020 16:13
`go mod init github.com/hound-search/hound`
Replaces Travis CI with Github Actions for better integration.
Jobs are run against more recent Go versions, namely 1.12 to 1.14 .
 .
For further documentation see

- https://github.com/actions/setup-go
- and https://help.github.com/en/actions

This adds `-race` flag to the Go tests and will run a meta linter
(golangci-lint).
We can use relative paths since this is a Modules project now.
We can safely assume that the application can be build when the test
suite succeeds.
The former package is unmainatained since a while and the replacement
also fixes this bug I encountered when trying to build the package:
jteeuwen/discussions#6
Tests will fail if the unmmap error is returned instead of being logged
(FIXME annotation).
@klingtnet
Copy link
Contributor Author

Nevermind, I implemented my own code search in the mean time (which will hopefully be open sourced as well).

@klingtnet klingtnet closed this Oct 1, 2020
@salemhilal
Copy link
Contributor

Hey, we appreciate this contribution. We're still trying to triage some of the issues in the backlog. It's very hard to review all these pull requests, particularly when we are still ramping up on our understanding of the codebase, and when the PRs are especially large (like this one). I'd be happy to merge your work, but I'd ask that you be patient with us as well.

If you want to make things a bit easier on our end, I'd love to see each of the features in this PR broken up into separate PRs (linting, Github Actions, the Go runtime upgrade, the migration to Go modules, and the enforcement of a linter).

@salemhilal salemhilal reopened this Oct 1, 2020
@klingtnet
Copy link
Contributor Author

Hey, we appreciate this contribution. We're still trying to triage some of the issues in the backlog. It's very hard to review all these pull requests, particularly when we are still ramping up on our understanding of the codebase, and when the PRs are especially large (like this one). I'd be happy to merge your work, but I'd ask that you be patient with us as well.

If you want to make things a bit easier on our end, I'd love to see each of the features in this PR broken up into separate PRs (linting, Github Actions, the Go runtime upgrade, the migration to Go modules, and the enforcement of a linter).

Thank you for the quick reply, I am happy to see that the project is not abandoned, which I thought it is previously.
I can definitely understand that splitting up the PR makes reviewing it a lot easier since it limits the scope of each change. This is also what I would usually do but since there wasn't much activity in the project I wanted to get as much as possible in the PR when it gets eventually merged.

However, I do not know when I find the time for splitting up or adjusting the changes. If anyone else want's to grab a part, e.g. adding Go modules support, feel free to do so.

@salemhilal
Copy link
Contributor

That makes sense. For some context, I'm one of a handful of engineers that have volunteered to maintain the project as of this summer:
#328 (comment)

We are definitely interested in making sure Hound is super stable so that adopting future contributions would be easier, so at the very least this PR goes a long way towards helping us carve out some work.

Base automatically changed from master to main February 24, 2021 17:03
@salemhilal
Copy link
Contributor

I'm closing this PR since its pieces, #353, #354, and #356, have been merged. I've opened #412 to track work towards automating Golang linting of this repo.

Thank you again for all of this work!

@salemhilal salemhilal closed this Jan 19, 2022
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.

2 participants