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

Add parameter return-license in build job #29

Merged
merged 6 commits into from
Sep 19, 2022
Merged

Add parameter return-license in build job #29

merged 6 commits into from
Sep 19, 2022

Conversation

tadashi0713
Copy link
Contributor

https://game.ci/docs/github/returning-a-license

Since Unity only allows returning professional licenses, I added a parameter to switch whether returning licenses or not for other types of licenses.

@EricRibeiro
Copy link
Member

Thanks, @tadashi0713 🙇

Weird that shellcheck is complaining since you didn't touch any script. Nevertheless, is a valid error that it caught, so I'll go ahead and open a PR with the fix and then we can merge yours.

@EricRibeiro EricRibeiro linked an issue Sep 12, 2022 that may be closed by this pull request
@nanodeath
Copy link

Speaking as a free Unity user, it'd be kinda nice if the default just worked. Can we always have this orb try to return the license, and fail gracefully and continue if it can't? Or is that a recipe for problems.

@EricRibeiro
Copy link
Member

Speaking as a free Unity user, it'd be kinda nice if the default just worked. Can we always have this orb try to return the license, and fail gracefully and continue if it can't? Or is that a recipe for problems.

I don't believe it's a problem in most cases. However, It would make your job run time unnecessarily longer. The doc linked by @tadashi0713 says:

Manually returning a license is usually never necessary, unless when running into an unrecoverable error while having a license active.

Also, Unity only allows returning professional licenses.

Meaning that even for Pro and Plus, it's not always necessary to return the license. Perhaps the best approach is to set the new parameter to false as the default. What do you all think @nanodeath and @tadashi0713?

@EricRibeiro
Copy link
Member

Also, @tadashi0713, can I ask you to include the new parameter in the test job? We will need it there, too, since it also uses the return-license command.

@EricRibeiro
Copy link
Member

Pushed the change with my suggestions and merged the shellcheck fix, let me know what you all think.

@nanodeath
Copy link

Looks good to me.

@tadashi0713
Copy link
Contributor Author

@EricRibeiro Thank you for your suggestion/fix, also looks good to me 👍

@KyleTryon
Copy link
Member

Looks like this resolves #26

@EricRibeiro EricRibeiro merged commit cc81411 into game-ci:main Sep 19, 2022
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

Successfully merging this pull request may close these issues.

Make return_license step in main job optional
4 participants