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 options to install source, docs, and examples #179

Merged
merged 5 commits into from
May 13, 2023

Conversation

ddalcino
Copy link
Contributor

Fix #87

The code for installing source is very similar to the code that installs examples and docs, so I went ahead and implemented all of them.

This PR probably includes more refactoring than it should, but I was careful to keep all the refactors in their own commits, in case you want to move them to a separate PR. This PR would be too tedious for me to write without using these refactors, so they're included anyway.

Please see the changes to the README.md for a discussion of all the new features added here.

Comment on lines 248 to 250
private static getBoolInput(name: string): boolean {
return core.getInput(name).toLowerCase() === "true";
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does anyone know where this Inputs.getBoolInput function came from? It looks like a duplicate of getBooleanInput(name: string, options?: InputOptions): boolean from @actions/core (see docs) to me. Should we be using that instead?

Copy link
Owner

@jurplel jurplel May 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This action is kinda old now, so it wouldn't surprise me if there is a new method now. Feel free to make a PR and fix that, but no problem for now.

@ddalcino ddalcino marked this pull request as ready for review March 19, 2023 04:10
@jurplel
Copy link
Owner

jurplel commented May 1, 2023

If you fix the merge conflict I will merge it.

Sorry for the delay, life at university is busy as always.

If a user wants to install source, docs, or examples, without installing
Qt binaries, then that user would have to turn on `tools-only`.
That option does not make semantic sense for these use cases, since the
user is not installing tools alone, and may not be installing tools at
all.

This change allows a user to turn off installation of Qt binaries using
an option that makes sense semantically.
This is meant to DRY-up some repetitive code, for readability and
maintainability benefits.
@ddalcino ddalcino force-pushed the topic/add-src-doc-examples branch from 474fa82 to 2a2a30b Compare May 1, 2023 23:28
@ddalcino
Copy link
Contributor Author

ddalcino commented May 4, 2023

If you fix the merge conflict I will merge it.

Sorry for the delay, life at university is busy as always.

@jurplel all merge conflicts are now resolved.

@jurplel jurplel merged commit b0a26e3 into jurplel:master May 13, 2023
jurplel added a commit that referenced this pull request Aug 4, 2023
Add options to install source, docs, and examples
@ddalcino ddalcino deleted the topic/add-src-doc-examples branch September 9, 2023 19:55
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.

Way to install source tree as well
2 participants