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

Houndd version #362

Merged
merged 7 commits into from
Oct 30, 2020
Merged

Houndd version #362

merged 7 commits into from
Oct 30, 2020

Conversation

kartikay101
Copy link
Contributor

Fixes #306
Display the version of houndd and exit

usage houndd -version

the version can be set while building
make VERSION=1.1.1 will set the version of houndd to 1.1.1

If no version is provided a default of 1.0.0 is used

@salemhilal
Copy link
Contributor

This is a good start! I think I'd prefer the version of the cli to be defined in the actual go runtime, rather than passed in as an environment variable at build-time. The version is more related to the contents of the code than when and how it's built.

I think the approach that gopass uses makes a lot of sense. They expose a semver version struct as the source of truth:
https://github.com/gopasspw/gopass/blob/6c94cbef056df696968a7cc4a8594b42c7318bb2/main.go#L253

It makes sense to me to expose the version in a similar way, either as a constant or through a getter function, in hound/cmds/houndd/main.go. Does that sound reasonable?

@kartikay101
Copy link
Contributor Author

kartikay101 commented Oct 26, 2020

That will be an easier thing to do, just setting 3 constants and returning a concat of those. The reason I chose build over that approach is because people tend to forget updating these and realize it a bit too late. Another thing that can be done in the current approach is to mandate version provision instead of defaulting it.

On second thought, the version of houndd should only change when there has been an actual feature/code change in it. Although this is doable in build as well, it requires extra effort over just keeping the constants.

A third alternative would be to setup a string that is over writable from the build scripts as well i.e. giving values through code or using build VERSION=something will be ways to change the version of houndd ( this seems a more preferable approach ).
Just realized this is what is done by gopass as well. I'll get started with this, should be done soon.

Thoughts/ comments ?

@kartikay101
Copy link
Contributor Author

Made the relevant changes.
Used similar structure to gopass. It passes semver.Version instead of a string

Ways to provide version are:

  1. Edit the main.go to change the version Major=2 Minor=5 Patch=3 results in 2.5.3 as the version.
  2. Provide it as VERSION flag in make. make VERSION=2.4.3 results in 2.4.3` as the version (which overrides the version given inside the main.go file)

To see the version use houndd -version

Minor change, displayed houndd v1.2.3 instead of just houndd 1.2.3

Copy link
Contributor

@salemhilal salemhilal left a comment

Choose a reason for hiding this comment

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

Thank you for the updates! I left two blocking comments.

I think my idea about how releases would work are that they're intentionally done as part of a release. Can you tell me when we'd want to manually override just the release version, but not its code, during the build process? I'm not saying that there isn't a way, but I'm just having a hard time seeing it. The downside is that we might end up with a build that is labeled as one version, but has the source code for another.

Relatedly, I think some work for me to do here is to document our release process so that the steps for upgrading are clear.

At any rate, I'm gonna nudge some of the other maintainers about their thoughts on versioning. Aside from my comments and my thoughts on passing in a version flag, I like this PR!

if err == nil {
return ver
}
return semver.Version{
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: The latest release is 0.3.0, technically. Are you seeing v1.0.0 listed somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will change it to 0.3.0

@@ -13,6 +14,7 @@ import (
"strings"
"syscall"

"github.com/blang/semver"
Copy link
Contributor

Choose a reason for hiding this comment

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

we've just added support for gomodules — are you ok with adding this to our go.mod file? I believe you will need to rebase master into this branch and force-push to see it.

@kartikay101
Copy link
Contributor Author

kartikay101 commented Oct 28, 2020

I agree with you, it isn't a good practice to have 2 sources of truth, thus the first approach ( only via build )

I agree with the idea to have it consistently available at both locations ( code + actual binary )

The only reason I went with the approach I took is to because the reference suggests so ( as they are using the version flag in multiple locations )
Let me get this sorted and push it.

P.S. Could you mark the pr as hacktoberfest-accepted in the tags.

@kartikay101
Copy link
Contributor Author

Add semvar to go.mod
Removed functionality add version through the makefile

Only way to provide versioning is via code i.e Major Minor And Patch format.

@kartikay101
Copy link
Contributor Author

Could I nudge you to either accept/ merge this PR or just add a label as hacktoberfest-accepted as that would help me get my hacktober event completed.

If there are any questions/ideas regarding this PR please feel free to reach out.

@salemhilal
Copy link
Contributor

@kartikay101 yes, totally. I planned on working through some of the backlog of issues today. I'll add the label for now, but I also hope to get this merged shortly as well. Thanks again for your contribution!

@salemhilal salemhilal added the hacktoberfest-accepted PRs with this label are accepted but not ready to merge, and count as a Hacktoberfest contribution label Oct 30, 2020
@salemhilal
Copy link
Contributor

After some local testing, this looks good to me! Thanks again, and I hope you snag that t-shirt!
If you are looking for another issue before the deadline, there's a short one that I just opened. You're welcome to it, but no pressure. I'll be working through some of the other pull requests before I try and implement that fix.

@salemhilal salemhilal closed this Oct 30, 2020
@salemhilal salemhilal reopened this Oct 30, 2020
@salemhilal salemhilal merged commit 06e14d9 into hound-search:master Oct 30, 2020
AlexFielder added a commit to AlexFielder/hound that referenced this pull request Jul 28, 2021
* Update config-example.json

* Add CONTRIBUTING.md file

* Dockerfile should use github.com/hound-search

This fixing the docker build issue.

* make listen URL clickable in iTerm (hound-search#340)

* Add Jest tests and Prettier support (hound-search#343)

This commit adds support for Jest and writes a few simple tests as
proof. Adding a test is as easy as adding a file that ends in
`.test.js` next to the file you want to test. Any file that matches that
pattern should have access to Jest functions. The whole test suite can
be run with `npm run test`.

I also added support for Prettier. Running `npm run format`
should run Prettier on any staged changes. I thought about adding Husky
to automatically run Prettier as a pre-commit hook, but it seems as
though Husky requires git v2.13 which would require me upgrading Git.

I didn't add support for ESLint yet since I figured that may be a
slightly more involved conversation — Prettier seems to be generally
uncontroverical and is generally only slightly configurable, whereas
ESlint is highly configurable and behaves however we want it to.

* Git VCS driver: Support non-default target refs (hound-search#345)

Currently, only the default ref (`master`) is supported.

- Allow target non-default target refs to be specified via configs.
- Add config-driven target ref auto-detection using the `HEAD` branch.
- Update default repo base URL pattern to link to the content revision,
  accounting for potential non-default target ref usage.
- Add global-level VCS configs which are merged into the repo-level configs
  during startup (repo-level vals take precedence).

* Exclude file path (hound-search#347)

* index: add a search option to exclude files given a regexp

* api: add the excludeFiles query parameter

Set the index search options ExcludeFileRegexp with this parameter
to allow excluding files from the search via a regexp.

* js: add the excludeFiles search parameter

* css: increase the label width to fit 'Exclude file path'

* js: correctly check that advanced is empty

* Update README with contact info for maintainers. (hound-search#348)

I don't think there's a clear way to see who has push privs to this repo from the public, and even if there is, the "Get In Touch" section should probably list the maintainers.

* incorporate detailing around gopath in README's quick start instructions (hound-search#352)

Co-authored-by: Rebecca Lau <[email protected]>

* pin alpine version to make the docker pass

* Update Dockerfile

* More detailed setup documents with less assumptions on users (hound-search#359)

This commit updates the readme to be a little more verbose for people who don't necessarily have Go or Docker set up, but who want to contribute nonetheless. 

* Apply the old patch

* add  some figures

* remove the idea code

* get it untitled

* remove

* add .idea dev tools

* remove simple config

* get config back

* detailing tests and simplifying the setup

* Commit TLS support guide

Co-authored-by: Salem <[email protected]>

* make the statement terser

* delete unused img

* rewording

* back to the old path

* Update README.md

* Update README.md

* Update README.md

* Update README.md

Co-authored-by: Salem <[email protected]>

* Initialize Go Modules

`go mod init github.com/hound-search/hound`

* Replace jteeuwen/go-bindata with go-bindata/go-bindata

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

* update Go version requirement in README file

* Add a -version flag to Houndd (hound-search#362)

This commit adds a `-version` flag to houndd. Running `houndd -version` prints the current version and exits.

Co-authored-by: Kartikay Shandil <[email protected]>

* Add padding between advanced mode text inputs

* add margin bottom to fields that are not last

* update front end bundles

Co-authored-by: Blobb Ered <@blobbered>

* Setup Github Actions CI (hound-search#354)

* Setup Github Actions CI

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).

* keep travis in until github workflows is tested

* tidy go mods

* remove lint command from github workflow

* add back go-bindata to go modules list

* up version of actions checkout to fix deprecation errors

* return go bindata to the go.sum file as well

Co-authored-by: Andreas Linz <[email protected]>

* Add a github actions badge

Thanks again @klingtnet and @blobbered for making this possible!

* Update README.md (hound-search#369)

Add clarification of using file:// protocol for accessing locally pulled repos

* Added new folder docs with config-options description on options. (hound-search#370)

* Added new folder docs with config-options description on options.

* Peer review comment update
updated formatting for config options documentation and added link
in readme.

* readme link update

* link update

* Minor edits from code review

Co-authored-by: kayyapil <[email protected]>
Co-authored-by: ehrktia <[email protected]>
Co-authored-by: Salem <[email protected]>

* Test windows

* Run Go build and test on Ubuntu Linux and Windows

* Add lint

* Install dependencies in Dockerfile

* Fix typo

* Adding a tip for windows users (hound-search#379)

* Adding a tip for windows users

* Update README.md

Co-authored-by: Salem <[email protected]>

Co-authored-by: Salem <[email protected]>

* Use tini as default entry point (hound-search#376)

* Add a warning for the default branch rename (hound-search#380)

* Add a warning for the default branch rename

* Add a link to Github's renaming documentation.

* Update README to reflect branch rename. (hound-search#384)

* Update version in main.go

* support github webhook

* Omit ports in constructed URL (hound-search#383)

* Improve accessibility (hound-search#398)

* Resolve WCAG2AA.Principle3.Guideline3_1.3_1_1.H57.2

* Resolve WCAG2AA.Principle4.Guideline4_1.4_1_2.H91.InputText.Name

* Resolve WCAG2AA.Principle4.Guideline4_1.4_1_2.H91.Button.Name

* Improve stats contrast

* Improve advanced text contrast

* Improve repo title contrast

* Improve contrast in advanced section

* Improve line number contrast

* Improve excluded files contrast

* Improve "load all" button contrast

* Automatically build docker image and publish to ghcr.io (hound-search#401)

* Feat: Use goreleaser

* Feat: Add docker build, remove goreleaser

* Fix: Add image name

* Feat: Use ghcr.io and build-push-action@2

* We try again

* Trying yet again

* Fix: Use actor and GITHUB_TOKEN

* Fix: Use github.actor in image tag path

* Fix: use github.repository_owner instead of github.actor

When you have an organisation, github.actor != github.repository_owner

Co-authored-by: Jone Marius Vignes <[email protected]>

* Update default-config.json (hound-search#391)

* Update default-config.json

* Update default-config.json (ref: main)

* Add hyperlinks to repository root (hound-search#396)

* Add literal search option (hound-search#397)

* Add literal search option

* Pull EscapeRegExp into common.js

* Add db/ to Jest ignore patterns

* Test EscapeRegExp() matches its input

* Test vacuous EscapeRegExp

* Give repo links a target of blank (hound-search#404)

Add rel="noopener noreferrer" to _blank links

Co-authored-by: Joseph Price <[email protected]>
Co-authored-by: dschott <[email protected]>
Co-authored-by: Nicolas R <[email protected]>
Co-authored-by: Igor <[email protected]>
Co-authored-by: Salem <[email protected]>
Co-authored-by: Richard Liang <[email protected]>
Co-authored-by: Vincent Rischmann <[email protected]>
Co-authored-by: Blobb Ered <[email protected]>
Co-authored-by: Rebecca Lau <[email protected]>
Co-authored-by: Ruixi Fan <[email protected]>
Co-authored-by: Ruixi Fan <[email protected]>
Co-authored-by: Andreas Linz <[email protected]>
Co-authored-by: Rebecca Lau <[email protected]>
Co-authored-by: Kartikay Shandil <[email protected]>
Co-authored-by: Kartikay Shandil <[email protected]>
Co-authored-by: Ashley Roach <[email protected]>
Co-authored-by: karthick <[email protected]>
Co-authored-by: kayyapil <[email protected]>
Co-authored-by: ehrktia <[email protected]>
Co-authored-by: David Kolossa <[email protected]>
Co-authored-by: Sandro <[email protected]>
Co-authored-by: Andrew Stanton <[email protected]>
Co-authored-by: David Schott <[email protected]>
Co-authored-by: Cor Bosman <[email protected]>
Co-authored-by: Jacob Rose <[email protected]>
Co-authored-by: Alexander Chiu <[email protected]>
Co-authored-by: Joel Armstrong <[email protected]>
Co-authored-by: Jone Marius Vignes <[email protected]>
Co-authored-by: Jone Marius Vignes <[email protected]>
Co-authored-by: Simon Legner <[email protected]>
Co-authored-by: Bucky Schwarz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted PRs with this label are accepted but not ready to merge, and count as a Hacktoberfest contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Is it possible to get houndd version ?
3 participants