Git Product home page Git Product logo

Comments (31)

MadhavJivrajani avatar MadhavJivrajani commented on June 21, 2024 4

I found something interesting:

Go allows injecting values into package-level variables at build time, try building this program using:

go build -ldflags="-X 'main.atBuild=lol'"

and on running, lol should print.

Since we already have a script that builds grofer executables before our releases, maybe we can modify that to take a version number and then build that into our executables.

For that, we would need to add a variable to the cmd package, preferably in about.go, something like:

...

// 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 grofer using:

go build -ldflags="-X 'github.com/pesos/grofer/cmd.groferVersion=<version>'"

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?

from grofer.

7wik-pk avatar 7wik-pk commented on June 21, 2024 4

ok, I know it has been nearly 2 weeks, but I got really busy, you know, life and sh*t
so apologies for that, but I'll definitely get this done (might need another 2 weeks lol, but will definitely do it at some point)
I'm caught up with too many obligations/tight deadlines atm, but I'll finish everything in time

from grofer.

MadhavJivrajani avatar MadhavJivrajani commented on June 21, 2024 2

@7wik-pk sorry for all the hastle and possible confusion, for this issue, you can go ahead and hardcode a version of v1.3.1-dev (the next release version) in the cmd/about.go file as a variable groferVersion as shown above.

If you're further interested in contributing to the versioning aspect of this, feel free to take up the issue that's created for:

script which checks for licenses, but is run only when you create a PR to stable, and checks if the groferVersion at the HEAD of stable is same as groferVersion at HEAD commit in PR, if so, exit with non-zero exit code, giving an appropriate error message.

from grofer.

MadhavJivrajani avatar MadhavJivrajani commented on June 21, 2024 2

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.

from grofer.

Gituser143 avatar Gituser143 commented on June 21, 2024 1

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:

  • This shows how we can use github.ref for container version tags
  • This explains what these variables are.

CC @Samyak2 this might help with container registry versioning too

from grofer.

Samyak2 avatar Samyak2 commented on June 21, 2024 1

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?

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 git describe to get a tag.

Running git describe --first-parent --tags --always --dirty on my branch of grofer gives:

v1.2.0-27-g0de6471

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

from grofer.

Samyak2 avatar Samyak2 commented on June 21, 2024 1

Here's an example of Go-based project that uses Make for building. The command git describe --always --tags used in their Makefile gives the same output

v1.2.0-40-g0de6471

from grofer.

Samyak2 avatar Samyak2 commented on June 21, 2024 1

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.

from grofer.

MadhavJivrajani avatar MadhavJivrajani commented on June 21, 2024 1

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:

  • When someone does a go get and runs grofer
    • This is the only weird case. Hardcoding is the only thing that comes to mind, will have to think a bit more on this
  • When someone builds from source
    • This is relatively straightforward because of the above discussion
  • When someone downloads the binary as a release artifact, maybe using something like curl
    • This is straightforward as well because of the script we have for creating release binaries.

from grofer.

Gituser143 avatar Gituser143 commented on June 21, 2024 1

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

This makes sense, go get does work with a version too, omitting gets latest I think. I could be wrong though.

from grofer.

MadhavJivrajani avatar MadhavJivrajani commented on June 21, 2024 1

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 stable, and checks if the groferVersion at the HEAD of stable is same as groferVersion at HEAD commit in PR, if so, exit with non-zero exit code, giving an appropriate error message.

We should create a new issue for this.

from grofer.

MadhavJivrajani avatar MadhavJivrajani commented on June 21, 2024 1

if you choose to go ahead with a config file, we won't need to hardcode a variable in the about.go file though.

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

I suppose now I just want to ask if I should be hardcoding a variable now or not lol

Yep, go ahead xD

from grofer.

7wik-pk avatar 7wik-pk commented on June 21, 2024 1

I have NOT tested grofer on my system yet because I use Windows for everything so I couldn't build it successfully.
Since it's an insignificant modification of the script, I'm hoping that's ok.
In any case, I'll set up a Linux VM soon and test it anyway.
For now though, please let me know if I somehow managed to break the code by adding three lines. That would be hilarious.
Thanks

from grofer.

7wik-pk avatar 7wik-pk commented on June 21, 2024

[Just to clarify] these "version numbers" must be extracted from a config file, (like .json or .yml) yes?
I only found one yaml file in the repo (inside the .circleci directory) and I wanted to enquire if I'm supposed to read that or if an alternative solution is expected by the maintainers.

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

from grofer.

MadhavJivrajani avatar MadhavJivrajani commented on June 21, 2024

Hey @7wik-pk! Welcome!
To answer your questions:

  • The version numbers in this context are typically ones that get tagged when a new release is made. For example, the tags can be seen in the Tags section:

Screenshot_20210523_164107

  • The config.yml file that you see holds the configuration of the Continuous Integration (CI) that runs when you create a PR or make a commit and is typically unrelated to grofer's version.

    • The CI's job is basically to ensure that the code that is committed does not contain errors and can be built successfully, and any tests that are written, are run and pass successfully.
    • In most cases, a PR is merged only if the CI completes successfully, however there are a few cases where the repo admins use admin privileges to merge a PR even if checks do not complete in the CI, this is typically due to a temporary glitch that occurs in the CI.
    • A successful CI would typically look like this (in grofer):
      Screenshot_20210523_164805
  • For adding a version in the about, the easiest way would be to hardcode it, but I feel like there might exist a smarter way to go about it, @Gituser143 did you have any specific method in mind or can we continue brainstorming?

from grofer.

Gituser143 avatar Gituser143 commented on June 21, 2024

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.

from grofer.

MadhavJivrajani avatar MadhavJivrajani commented on June 21, 2024

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 😄

from grofer.

7wik-pk avatar 7wik-pk commented on June 21, 2024

Sure, thanks
but I had a proposal: why not make a yaml/json or any other kind of config file for this project? that's the only thing I'm familiar with when it comes to keeping track of things like version/build (and even credits in some games)
If there's an alternative to that, I'd be happy to look into it

from grofer.

Gituser143 avatar Gituser143 commented on June 21, 2024

This is great, I like this.

from grofer.

MadhavJivrajani avatar MadhavJivrajani commented on June 21, 2024

This is the only weird case. Hardcoding is the only thing that comes to mind, will have to think a bit more on this

One thing that comes to mind is something like, if the groferVersion variable is non-empty, only then add a version to the about paragraph. But not sure how I feel about this because of it being inconsistent

from grofer.

Samyak2 avatar Samyak2 commented on June 21, 2024

Does go get use the latest main/master branch or the latest release?

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 1.2.0-dev (or whatever the current version is) by default. Although this will need to be updated on every release which is not very bad xD

from grofer.

MadhavJivrajani avatar MadhavJivrajani commented on June 21, 2024

Waaaaaaaaaiiiiiiiiiiiiiiiiiiiiiiit a minute
You can do -ldflags with go get as well 🙃
We can document it in the README

from grofer.

Samyak2 avatar Samyak2 commented on June 21, 2024

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

from grofer.

7wik-pk avatar 7wik-pk commented on June 21, 2024

because you'd might as well just hardcode the version, because we'd have to manually commit a change to update version anyway.

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)

from grofer.

7wik-pk avatar 7wik-pk commented on June 21, 2024

for this issue, you can go ahead and hardcode a version of v1.3.1-dev (the next release version) in the cmd/about.go file as a variable groferVersion as shown above.

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

If you're further interested in contributing to the versioning aspect of this, feel free to take up the issue that's created....

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.

from grofer.

MadhavJivrajani avatar MadhavJivrajani commented on June 21, 2024

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

Hmm interesting, thanks for sharing this! Might be worth looking into it in the future.

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

Can you elaborate? Leave it like how?

from grofer.

7wik-pk avatar 7wik-pk commented on June 21, 2024

Can you elaborate? Leave it like how?

I meant we might as well not add version to about.go for the moment because

  1. Hardcoding is not going to be a good solution
  2. Someone (probably myself) is going to replace it with a better solution in the future anyway, so in a way there's no point

from grofer.

Samyak2 avatar Samyak2 commented on June 21, 2024

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 browser-history I have to change the version in 3 places before a release xD. @7wik-pk your idea of having a config file should help there.

from grofer.

7wik-pk avatar 7wik-pk commented on June 21, 2024

As a comparison, in browser-history I have to change the version in 3 places before a release xD. @7wik-pk your idea of having a config file should help there.

if you choose to go ahead with a config file, we won't need to hardcode a variable in the about.go file though.
we could pull it from the config file inside the about.go script.

I suppose now I just want to ask if I should be hardcoding a variable now or not lol

from grofer.

Samyak2 avatar Samyak2 commented on June 21, 2024

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.

from grofer.

7wik-pk avatar 7wik-pk commented on June 21, 2024

(just wanted to let you guys know, I didn't intend to just disappear without any acknowledgement)

from grofer.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.