-
Notifications
You must be signed in to change notification settings - Fork 52
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
[FEATURE REQ] Add version number to about page #117
Comments
[Just to clarify] these "version numbers" must be extracted from a config file, (like .json or .yml) yes? [also, it seemed odd to me that the config.yml file exists in a directory instead of being in a "config" folder or the repository, outside everything. Made me think that maybe that file has a different purpose that's counter-intuitive to me.] Thank you. |
Hey @7wik-pk! Welcome!
|
I haven't really thought about how to go about this. But if we do come up with a solution, it should account for future releases. They should automatically have version updated, rather than manually have made a commit post/pre release. |
Alright, I will try and do a little research on it as well. @7wik-pk I'll assign this to you and we'll help you get a PR raised and merged! Feel free to post any and all doubts/questions you may have here or on the #grofer-help slack channel or feel free to DM us if you're more comfortable with that 😄 |
Sure, thanks |
I'm not sure a config file would be the way to go, because you'd might as well just hardcode the version, because we'd have to manually commit a change to update version anyway. For now I'm thinking a simple hardcoding might suffice, but an action could probably be setup to extract version from the tag maybe (Might be better to go over this in a separate issue)? I'm not very sure about this. My references are:
CC @Samyak2 this might help with container registry versioning too |
I found something interesting: Go allows injecting values into package-level variables at build time, try building this program using:
and on running, Since we already have a script that builds For that, we would need to add a variable to the ...
// groferVersion is the version of grofer that is loaded in during build
var groferVersion string
// aboutCmd represents the about command
var aboutCmd := &cobra.Command{...}
... and then build
This would work great for making releases, but not so much if someone is building from source, but documentation/build scripts for that could be provided? What do y'all think? |
This is great, I like this. |
For versioning at build time, would it possible to get the last commit's hash? For example, neovim hardcodes the version number if not in a git repo, otherwise it uses Running
We could use this as the version. This will need a build script or a Makefile for building though (or a really long command xD). |
Here's an example of Go-based project that uses Make for building. The command
|
Sorry for all the notifs, here's a better example of dgraph's Makefile. They too use build time variables (which @MadhavJivrajani detailed above) here. |
Yeah, that's exactly what I had in mind, but didn't know the commands :p Thanks for the links! So, we would have 3 types of scenarios:
|
One thing that comes to mind is something like, if the |
Does I don't like the idea of omitting version when installed from go get. One of the reasons I want version info in the about page is to ensure that go get updated grofer correctly (and I'm running the correct/latest version of it). One alternative is hardcoding it to |
This makes sense, go get does work with a version too, omitting gets latest I think. I could be wrong though. |
Waaaaaaaaaiiiiiiiiiiiiiiiiiiiiiiit a minute |
That's nice, but the issue is that we'll also have to update the README on every release to change the command xD Edit: if we're doing that, why not hardcode it in the code itself xD |
Yeah lol fair enough. Although I'd want a safeguard for not forgetting to update it when a release is made, something like the script which checks for licenses, but is run only when you create a PR to We should create a new issue for this. |
@7wik-pk sorry for all the hastle and possible confusion, for this issue, you can go ahead and hardcode a version of If you're further interested in contributing to the versioning aspect of this, feel free to take up the issue that's created for:
|
yes, we would have to hardcode it in the yaml file, but I was thinking that in the future you could create more packages/scripts/programs that will make use of version, and hardcoding it in one place (yaml file) and pulling it from there everywhere you need to use it would be better than hardcoding everywhere : this is what I was thinking at the time but yes, you might as well create a package-level variable (or even export one) in the about.go file instead, and pull it from here everywhere else, but I found this tool that automatically updates a json file whenever someone releases a new build (compatible with major & minor) so you could create a config file once and have something like this update it automatically in the future (apologies for replying this late, I happen to be nocturnal) |
that's ok, someone will be doing it the smarter way in the future anyway, so we might as well leave it like that for now
interested, yes, but I'm a little preoccupied rn and this seems big, I will need to learn a few things (I still need to familiarize myself with Go lmao) before I can handle this, so sometime in the future, I will either take this issue (if still open) or something else. |
Hmm interesting, thanks for sharing this! Might be worth looking into it in the future.
Can you elaborate? Leave it like how? |
I meant we might as well not add version to about.go for the moment because
|
Should it be replaced in the future by a better method, it can be an enhancement. However till that time comes, imo we still need the version number. Hardcoding isn't a bad idea if we have the nescessary automation and checks in place, for which the other issue will be raised. |
I agree with @MadhavJivrajani, hardcoding it as a variable is not such a bad idea. Though, it should be documented in the release process. As a comparison, in |
if you choose to go ahead with a config file, we won't need to hardcode a variable in the about.go file though. I suppose now I just want to ask if I should be hardcoding a variable now or not lol |
We would need to hardcode it in the config. Even if we try out the tool you linked, we would have to give write access to a bot-type thing, and then deploy that, which would have costs, I feel like that's a little over complicating things. Instead, a script to check in the CI itself if the version number is updated or no is pretty straightforward imo and robust enough to sustain
Yep, go ahead xD |
Sorry for the confusion, I was trying to show that hardcoding a variable is not a bad idea. You should go ahead with doing that. Harcoding + the script @MadhavJivrajani mentioned should be good enough for a long time. |
ok, I know it has been nearly 2 weeks, but I got really busy, you know, life and sh*t |
(just wanted to let you guys know, I didn't intend to just disappear without any acknowledgement) |
I have NOT tested grofer on my system yet because I use Windows for everything so I couldn't build it successfully. |
Is your feature request related to a problem? Please describe.
Currently,
grofer about
just has some text. It should additionally have version numbers too for each version built.Describe the solution you'd like
Adding version number to about. :P
The text was updated successfully, but these errors were encountered: