nmfs-ost / ss3-source-code

The source code for Stock Synthesis (SS3).
https://nmfs-ost.github.io/ss3-website/
Creative Commons Zero v1.0 Universal
37 stars 16 forks source link

Change version info in github action #202

Closed k-doering-NOAA closed 3 years ago

k-doering-NOAA commented 3 years ago

Do this to make it clear that the github action job is a tester version.

Note we will probably want a separate job for exes that are formally released (releases and distributed fixes)

kellijohnson-NOAA commented 3 years ago

I would think that you could do something like this bumping of version number using github actions where how the version number changes could be based on branch name.

Rick-Methot-NOAA commented 3 years ago

This would create a lot more MINOR and PATCH increments in between releases and it begs question of whether what we label as a "release" is automatically a MAJOR increment. So far, MAJOR is stuck at .30 and I do sometimes think about what would constitute .31. Conceptually, 4.0 will be FIMS, but I'll refrain from calling FIMS SS4.00.00.00

kellijohnson-NOAA commented 3 years ago

Right @Rick-Methot-NOAA, I agree that all branches don't need a version number. In my rainbows and lollipops world of thinking, you could have many feature branches, then when features are known to be part of a version you would merge those into a v_ branch named v_whateveryouwantafterunderscore. And, github action would realize that it is supposed to be a new version so it would label the executable with an incremental number with a note for pre-release. Then when the pull request was approved it could remove the "pre-release" note. None of this has to be done or investigated. Just thoughts.

k-doering-NOAA commented 3 years ago

Thanks @kellijohnson-NOAA and @Rick-Methot-NOAA ! These are all great thoughts. I really like the idea of making more things automated.

I was thinking maybe everything would just be labeled experimental (maybe with the last commit number), unless it is tagged and in the main branch? That way, when we are ready to do a release, we can just add the tag manually, as we normally do (if it is a prerelease, maybe we put that in the tag?) and the github action will take care of the rest? This isn't quite as automated as what Kelli is proposing, but I think it would basically allow us to work in the same way while automating this extra piece (the version numbering).

k-doering-NOAA commented 3 years ago

This would create a lot more MINOR and PATCH increments in between releases and it begs question of whether what we label as a "release" is automatically a MAJOR increment. So far, MAJOR is stuck at .30 and I do sometimes think about what would constitute .31. Conceptually, 4.0 will be FIMS, but I'll refrain from calling FIMS SS4.00.00.00

Good point...I wonder if bumping the major version would indicate to folks a big I/O change, which we (thankfully) haven't had? So perhaps given the way we have been doing versioning, it makes sense to be stuck at .30?

k-doering-NOAA commented 3 years ago

@Rick-Methot-NOAA , how should we implement this? Here are two options I am thinking about, but feel free to suggest another

  1. by default, says experimental In the files,
    #V3.30.experimental; not an official version of SS;_safe;...
  2. by default, says a placeholder, like 3.30.xx.yy. In the files,
    #V3.30.xx.yy;_safe;...

I think this matters because this is what would show up when compiling locally. I'm leaning towards 1, but what are your thoughts?

Rick-Methot-NOAA commented 3 years ago

I already am doing something like (1), but using beta rather than experimental.

3.30.17.beta

but should it be beta developments after .17

or 3.30.18beta as a beta for .18 in development?

On Wed, Sep 8, 2021 at 12:46 PM Kathryn Doering @.***> wrote:

@Rick-Methot-NOAA https://github.com/Rick-Methot-NOAA , how should we implement this? Here are two options I am thinking about, but feel free to suggest another

  1. by default, says experimental In the files,

V3.30.experimental; not an official version of SS;_safe;...

  1. by default, says a placeholder, like 3.30.xx.yy. In the files,

V3.30.xx.yy;_safe;...

I think this matters because this is what would show up when compiling locally. I'm leaning towards 1, but what are your thoughts?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nmfs-stock-synthesis/stock-synthesis/issues/202#issuecomment-915521296, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPV4ICYYMW2ZNFYNW2PFS3UA64SXANCNFSM5C6HZZEQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

k-doering-NOAA commented 3 years ago

Hmm, good point! I think leaving it as 3.30.17.beta makes sense given how we number fixes currently.

I was hoping to take all numbers except 3.30 out of the file and have github actions automate the other numbers for us; but this would mean any locally compiled exes would be missing the .17 (it would either look like 3.30.beta or 3.30.xx.yy). Does this seem ok? (EDIT: and if so, which one seems better?)

The other option is we could leave in the .17 and would just manually change the file upon every release. I don't like this as much, but we could do it.

Rick-Methot-NOAA commented 3 years ago

3.30.beta seems very ambiguous if we are to send it to someone for testing.

Richard D. Methot Jr. Ph.D.NOAA Fisheries Senior Scientist for Stock Assessments

Mobile: 301-787-0241

On Wed, Sep 8, 2021 at 12:58 PM Kathryn Doering @.***> wrote:

Hmm, good point! I think leaving it as 3.30.17.beta makes sense given how we number fixes currently.

I was hoping to take all numbers except 3.30 out of the file and have github actions automate the other numbers for us; but this would mean any locally compiled exes would be missing the .17 (it would either look like 3.30.beta or 3.30.xx.yy). Does this seem ok?

The other option is we could leave in the .17 and would just manually change the file upon every release. I don't like this as much, but we could do it.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nmfs-stock-synthesis/stock-synthesis/issues/202#issuecomment-915528235, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPV4IHVL7R3WIPYSXGIMO3UA654VANCNFSM5C6HZZEQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

k-doering-NOAA commented 3 years ago

yes, that is true... I am thinking the github actions could add in the .17, since we more and more have been sending exes compiled by it?

k-doering-NOAA commented 3 years ago

...and the github actions would get this from the last tag in the repo, so wouldnt be hard coded anywhere.

k-doering-NOAA commented 3 years ago

Of course, this ended up being a bit more of a rabbit hole than expected! I have automatic versioning added in all the jobs except for the linux one (having some issues because I need to use centos 8 for a new version of git, but then admb will only work on centos 7. Still working to solve it). This is done by editing the tpl file right before compiling based on the current commit and the last tag made. Here is how the automatic versioning works currently:

  1. The line in the tpl is #v3.30.xx.yy. This is how it will look if you compile locally without modifying the file.
  2. If the last commit made is tagged with the form v3.30.xx.yy, v3.30.xx, or v3.30.xx-prerel (for prereleases), this commit will be added to the version info. For example, if the last commit was tagged as v3.30.18, then the line in the version info would read #v3.30.18.00. If the tag is v3.30.18-prerel, then th line in the version info would read #v3.30.18.00-prerel.
  3. If the last commit made is tagged with something of another form, "unknown" will be appended. For example, if the last commit was tagged as my_bad_tag , the line in the version info would read #v3.30.unknown.unknown we could change this if there was a valid reason for wanting to use other tags.
  4. If the last commit made does not have a tag associated with it, only the .xx will be used from the last tag made, and beta will be appended. For example, if the last tag is v.30.30.17.02, but the tag is on an earlier commit than the current one, then the line in the version info would read #v3.30.17.beta

@Rick-Methot-NOAA , let me know if this needs any tweaks?

I will submit a PR once I figure out a solution to the centos build.

Rick-Methot-NOAA commented 3 years ago

This seems quite complex if it needs to interact with tags. Currently tags are only added for releases. So I am missing a piece of the logic. The changes being proposed here seem to supercede the changes already in a branch for adding DATE. Has that branch already been merged into main?

On Fri, Sep 10, 2021 at 10:09 AM Kathryn Doering @.***> wrote:

Of course, this ended up being a bit more of a rabbit hole than expected! I have automatic versioning added in all the jobs except for the linux one (having some issues because I need to use centos 8 for a new version of git, but then admb will only work on centos 7. Still working to solve it). This is done by editing the tpl file right before compiling based on the current commit and the last tag made. Here is how the automatic versioning works currently:

  1. The line in the tpl is #v3.30.xx.yy. This is how it will look if you compile locally without modifying the file.
  2. If the last commit made is tagged with the form v3.30.xx.yy, v3.30.xx, or v3.30.xx-prerel (for prereleases), this commit will be added to the version info. For example, if the last commit was tagged as v3.30.18, then the line in the version info would read #v3.30.18.00. If the tag is v3.30.18-prerel, then th line in the version info would read #v3.30.18.00-prerel.
  3. If the last commit made is tagged with something of another form, "unknown" will be appended. For example, if the last commit was tagged as my_bad_tag , the line in the version info would read

    v3.30.unknown.unknown we could change this if there was a valid

    reason for wanting to use other tags.

  4. If the last commit made does not have a tag associated with it, only the .xx will be used from the last tag made, and beta will be appended. For example, if the last tag is v.30.30.17.02, but the tag is on an earlier commit than the current one, then the line in the version info would read #v3.30.17.beta

@Rick-Methot-NOAA https://github.com/Rick-Methot-NOAA , let me know if this needs any tweaks?

I will submit a PR once I figure out a solution to the centos build.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nmfs-stock-synthesis/stock-synthesis/issues/202#issuecomment-917067807, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPV4IDQECCHUN2QQFDSPP3UBI3TXANCNFSM5C6HZZEQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

k-doering-NOAA commented 3 years ago

I'm thinking going forward we should also add some tags for any fixes that we post for the community? I did this for the last one (https://github.com/nmfs-stock-synthesis/stock-synthesis/releases/tag/v3.30.17.01).

My logic was we just call anything that we don't want to distribute to the community .beta, instead of giving a fix number. I think this is sort of what we are doing already. We could also append a last commit number, if that would be helpful.

I didn't change anything about the date formatting, just the version number. So unless the code for adding date also changes version number, I think this proposal is separate.

The changes are in a branch: https://github.com/nmfs-stock-synthesis/stock-synthesis/tree/gh_actions . Note I plan on cleaning this up (squashing commits), before submitting a pull request, as testing out things on github actions is a lot of trial and error.

k-doering-NOAA commented 3 years ago

Also, here is a link to a working file, that just builds the one windows exe: https://github.com/nmfs-stock-synthesis/stock-synthesis/blob/8a55415fc246d5c1845f1fe950ecdffc53e76144/.github/workflows/build-win-basic.yml . Pulling the information from tags is actually only 2 lines of code; most of what needed to be written was logic statments in R to format the version as desired.

Rick-Methot-NOAA commented 3 years ago

agree with tagging things that are released to the community.

the code I am working now with Nathan and Max for predators should be labelled beta. Right? But it is just to them and not to the public.

I have not been attentive to keeping a branch that is only fixes to the last release separate from a branch that is implementing new features. All are getting merged into main and it is getting released to public with this co-mingling.

Richard D. Methot Jr. Ph.D.NOAA Fisheries Senior Scientist for Stock Assessments

Mobile: 301-787-0241

On Fri, Sep 10, 2021 at 11:31 AM Kathryn Doering @.***> wrote:

Also, here is a link to a working file, that just builds the one windows exe: https://github.com/nmfs-stock-synthesis/stock-synthesis/blob/8a55415fc246d5c1845f1fe950ecdffc53e76144/.github/workflows/build-win-basic.yml . Pulling the information from tags is actually only 2 lines of code; most of what needed to be written was logic statments in R to format the version as desired.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nmfs-stock-synthesis/stock-synthesis/issues/202#issuecomment-917121646, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPV4IGHH4J5EMBXAJNNDJTUBJFINANCNFSM5C6HZZEQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

k-doering-NOAA commented 3 years ago

the code I am working now with Nathan and Max for predators should be labelled beta. Right? But it is just to them and not to the public.

Yes, that is what I was thinking! And that is what would happen in the github action. Basically, if it is not tagged, it's labeled beta.

I have not been attentive to keeping a branch that is only fixes to the last release separate from a branch that is implementing new features. All are getting merged into main and it is getting released to public with this co-mingling.

Yeah, I remember we discussed this a while back! I think it is ok as-is. We would need a lot more branches and rules on what to put where and how to merge them to keep everything separate. In my opinion, right now it isn't worth the extra effort at this moment. I think keeping new development in branches until we've tested it out a bit has gone a long way to make sure the new features are somewhat stable before they get into a fix incidentally.

k-doering-NOAA commented 3 years ago

I should add, the solution to getting the linux version was Johnoel was able to replace the admb 12.3 exe so that it would also work with gcc-8 (which was needed to get the linux job working in a centos 8 container). That solved the issue.