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

output installed version number after setup #51

Merged
merged 2 commits into from
Mar 9, 2020

Conversation

rdaumann
Copy link
Contributor

@rdaumann rdaumann commented Jan 8, 2020

implements #50

Example run with changes: Actions

@sburns
Copy link

sburns commented Jan 16, 2020

I'd like to see this go a tad further and write the version as an action output. This way, we should be able to reference that output in later steps. Specifically, I'd like to be able to build a virtualenv cache key based on the installed version of python.

@rdaumann
Copy link
Contributor Author

What format should the output have? Like in the install dir e.g. Python/3.8.0 or like in the console output Python 3.8.0?

@sburns
Copy link

sburns commented Jan 17, 2020

Personally I'd prefer no slashes or spaces. 3.7.6, 3.6.9, 3.8.0, pypy2, etc. The goal is to relay the specific version installed for cases that we ask for a non-specific version, like 3.7.X.

@rdaumann
Copy link
Contributor Author

rdaumann commented Jan 17, 2020 via email

await finder.findPythonVersion(version, arch);
const installed = await finder.findPythonVersion(version, arch);
console.log(
`Successfully setup ${installed.impl} (${installed.version}).`
Copy link
Collaborator

Choose a reason for hiding this comment

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

The extra . seems unnecessary

Suggested change
`Successfully setup ${installed.impl} (${installed.version}).`
`Successfully setup ${installed.impl} (${installed.version})`

@konradpabjan
Copy link
Collaborator

Tested out your changes, looks good 👍

Better output/logging is always good and this is a pretty small addition. Example output can be seen here: https://github.com/konradpabjan/Testing2/runs/489889083?check_suite_focus=true

@brcrista @madhurig any thoughts or objections?

Copy link

@madhurig madhurig left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for your contribution @robindaumann.

Thanks for testing @konradpabjan.

@konradpabjan
Copy link
Collaborator

Thanks for the contribution @robindaumann

I'll go ahead and merge your changes in. The small update that I suggested I'll address myself before pushing out a new release

@konradpabjan konradpabjan merged commit f8fb48e into actions:master Mar 9, 2020
konradpabjan added a commit that referenced this pull request Mar 9, 2020

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
* Use ncc instead of saving node_modules

* Add branding and correctly point to main file

* Cleanup

* Update release script

* PR Feedback

* Update README.md

* Update README.md

* Update contributors.md

* Update description

* use node-version instead of version (deprecated)

* Update contributors.md

* Update README.md

* Update README.md

* Create yaml-lint-config.yml

* Create lint-yaml.yml

* Update README.md

* Update README.md

* Update contributors.md

* Update README.md

* Update terminology in comments

* Spelling & grammar

* Consistent file name references

* ncc build

* Address YAML linting errors

* Fix quotes

* Bump handlebars from 4.1.2 to 4.5.3

Bumps [handlebars](https://github.com/wycats/handlebars.js) from 4.1.2 to 4.5.3.
- [Release notes](https://github.com/wycats/handlebars.js/releases)
- [Changelog](https://github.com/wycats/handlebars.js/blob/master/release-notes.md)
- [Commits](handlebars-lang/handlebars.js@v4.1.2...v4.5.3)

Signed-off-by: dependabot[bot] <support@github.com>

* Run main workflow on PRs

* Match README to action.yml

* Update dist/index.js

* Update checkout action to v2

* Fix cross-platform build matrix example

* output installed version number after setup (#51)

* output installed version number after setup

* set output for the installed version

* Setup python + self hosted runners documentation

* Updates to npm packages (#66)

* npm package updates

* Updates to ncc build

* Update action.yml

* Update action.yml

Co-authored-by: Konrad Pabjan <Konrad.Pabjan@microsoft.com>
Co-authored-by: Edward Thomson <ethomson@edwardthomson.com>
Co-authored-by: conao3 <conao3@gmail.com>
Co-authored-by: Brian Cristante <brcrista@microsoft.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Rui Chen <chenrui333@gmail.com>
Co-authored-by: Madhuri Gummalla <madhurig@github.com>
Co-authored-by: Ye-hyoung Kang <keepyourhonor@gmail.com>
Co-authored-by: Robin Daumann <26201853+robindaumann@users.noreply.github.com>
@rdaumann
Copy link
Contributor Author

rdaumann commented Mar 9, 2020

Alright thanks :)

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.

None yet

4 participants