-
-
Notifications
You must be signed in to change notification settings - Fork 33
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
fix dockerRepoVersion #99
Conversation
Activation got reworked with recent release (v4) of builder and test-runner actions. I'm not sure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure we want to merge this. This repo needs to be updated to print a message directing users to follow updated activations instructions and error out as this cannot be used to get license files anymore and then have the repo put into public archive. Updating the images means it will be more likely to inadvertently work and confuse new users following old documentation.
I could agree with either of your statements. On the one hand, it's not guaranteed that the old way of activating is completely obsolete for everyone. In which case @GabLeRoux is probably right that we should maybe naively upgrade it. At the same time it might make sense to put a disclaimer that there's a likeliness one wants to use the updated mechanism instead. Perhaps a hybrid solution between the two? WDYT @AndrewKahr? |
@webbertakken So there's a few reasons I'm recommending scorched earth for this action but leaving the other activate/return license actions alone:
The other activate/return license steps are automated and do more than our normal activation flow so I wouldn't want to impact those at all, however this repo effectively serves no role other than potential confusion to new users. Hence I recommend we simply empty the action out, throw a |
Perhaps primarily, perhaps not even that anymore. But there are people that don't use GameCI workflows, just this action and the return license. So we can not just drop it completely because the mainstream use case has changed. As there literally are thousands of projects using our actions, I want to make sure that we don't cut off people that were still using it. If you're sure we're not cutting off anyone then I'm fine, but otherwise I prefer a more hybrid approach. Perhaps even adopt the same activation as we do in builder.
I agree with everything you said before, but I don't agree that people don't read warnings, especially when just setting up a workflow. We should use best practices and use warning and error levels appropriately. |
I think there might be some confusion over the function of this repo. This repo is only to generate the (now defunct) alf file for the web tool Unity provides. But since Unity is removing the ability to use that web tool, that is why I don't think we should continue allowing this action to function as it will lead to more user confusion. Even for people who use old versions of unity-builder or their own tooling, it doesn't help them to generate the alf since the Unity side of the equation has now been removed which makes the alfs unusable. The activate/return actions will be left alone as those continue to provide value (and actually function). However, what I think we can do to be safe is merge this PR and tag it as v2.1, then merge a following PR that empties the action except for an error message with guidance on activation and tag it as v2.2 such that it becomes the latest for v2. That way new people following old docs will get v2.2 with guidance and if people have a need they can pin to v2.1 and open an issue so we can understand what that need is. Then when it's clear that nobody has any issues without the action (maybe wait 6 months), we can update the readme indicating the action is indeed unused and set the repo to public archive. Thoughts on this strategy @webbertakken? |
Sounds good. Let's do it! |
Changes
1
to3
.Checklist