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

'v1' release branch or tags to "release" an action #6

Closed
bryanmacfarlane opened this issue Oct 23, 2019 · 9 comments
Closed

'v1' release branch or tags to "release" an action #6

bryanmacfarlane opened this issue Oct 23, 2019 · 9 comments
Assignees

Comments

@bryanmacfarlane
Copy link
Member

Should the walkthrough take the simple approach of a v1 branch or have a release branch where we push the tag along?

Also, we need to update the toolkit versioning doc and update the link here.

There's also the option of the simple approach in the walk through but then reference the versioning doc for more robust and complex options.

@bryanmacfarlane
Copy link
Member Author

based on CR feedback from @jeffmcaffer - thanks!

@wesleytodd
Copy link
Contributor

I am not sure about this approach, but I was thinking about pulling this out into an action of it's own. Thoughts?

https://github.com/wesleytodd/meeting-maker/blob/master/test/_release.js

What it does is create a single commit off of a version tag being pushed, then builds and commits specifically the dist directory. This keeps the working branches clean of the build files, but still allows for the tagged release lines. If you think this approach is a good idea I can put together that action this week.

@jasonkarns
Copy link

@wesleytodd i'm becoming a bit disillusioned by that approach at present. Because actions don't yet support a "use-time compilation", having the compilation done in a script that only runs during a release flow makes it exceedingly more difficult for users to reference an unreleased action (ie, master) or to run off a fork.

I do think this setup would be ideal in a future where actions has its own automatic compilation (for instance, running npm install within the cloned action), but at present, I think this would just add more friction.

@bryanmacfarlane
Copy link
Member Author

bryanmacfarlane commented Apr 13, 2020

@jasonkarns - see actions/javascript-action, actions/setup-go, actions/setup-node etc. etc. They use ncc pack and there's no problem with master vs distribution branch. They're all the same with a dist folder. We won't be doing npm install at runtime - fragility, security (ref is what was reviewed) and we have GHES air gapped scenarios etc. What runs is an entry point and it has to be self container. ncc pack gives you that and you can ignore node_modules in every branch.

Also see the workflow tests in this repo and those which reference ./ which works on a push to master as well.

Finally, we will be adding actions in GPR (a new type of package) at some point in the future (medium term backlog) as well.

@jasonkarns
Copy link

@bryanmacfarlane Ah, my mis-understanding was that @wesleytodd's proposal would have the dist folder only exist outside of master (ie, built only off tags). I didn't realize the intention was for that workflow to commit back to the main branches.

mea culpa

@wesleytodd
Copy link
Contributor

Ah, my mis-understanding was that @wesleytodd's proposal would have the dist folder only exist outside of master (ie, built only off tags)

This is my intent. I will not be committing node_modules or other build files to master. If this continues to be the documented way it will turn many folks off. It certainly is not something I will be doing.

That said, I totally understand that this approach makes it more difficult to test builds. I think we can have the best of both worlds here with semver pre-release version. Push a tag like 1.0.0-beta-1 and all I would need is for that to be recognized in the action to add a tag to the build commit. I would be happy to add this feature to the existing implementation before pulling it out into a separate version.

Finally, we will be adding actions in GPR

Can you explain more about this? Would it follow semver without us having this sort of complicated setup in our actions?

@jasonkarns
Copy link

jasonkarns commented Apr 14, 2020

I will not be committing ... or other build files to master.

This includes dist/? If that's the case, then it becomes impossible for a workflow to leverage (or validate, or experiment with) unreleased features of an action. That's precisely the kind of friction I would hope is avoided.

To the original question:

or have a release branch where we push the tag along?

I'm personally very strongly in favor of only using a vN branch. git-tags are not supposed to move, and I think it communicates a lot of bad practices to suggest a "blessed" workflow that involves force-pushing tags. (IMO)

@wesleytodd
Copy link
Contributor

I am not sure I understand the reluctance to move tags, but if that is an issue for you we could do semver patterned branches just like the tags. I am not strongly opinionated on this part and both are trivial to implement.

Skitionek referenced this issue in Skitionek/notify-microsoft-teams Apr 23, 2020
Change default value of slack channel
@hross
Copy link
Contributor

hross commented Jul 17, 2020

closing this since it is old and right now we are pushing tags forward. will reopen if we decide to change that approach. At this point I think we still should commit dist to master (which we do) and I agree about the point of "hey we can't test unreleased features unless master has dist".

@hross hross closed this as completed Jul 17, 2020
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

No branches or pull requests

4 participants