8000 Update terminology in setup-python by brcrista · Pull Request #46 · actions/setup-python · GitHub
[go: up one dir, main page]

Skip to content

Update terminology in setup-python #46

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

Merged
merged 8 commits into from
Dec 30, 2019
Merged

Update terminology in setup-python #46

merged 8 commits into from
Dec 30, 2019

Conversation

brcrista
Copy link
Contributor

Some of the comments are using outdated terminology. I also took the chance to touch up the contributor guidelines.

@@ -34,7 +34,7 @@ module.exports =
/******/ // the startup function
/******/ function startup() {
/******/ // Load entry module and return exports
/******/ return __webpack_require__(429);
/******/ return __webpack_require__(264);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ran with version 0.20.5 of ncc.

action.yml Outdated
author: 'GitHub'
inputs:
python-version:
description: 'Version range or exact version of a Python version to use, using semvers version range syntax.'
description: 'Version range or exact version of a Python version to use, using SemVer's version range syntax.'
Copy link
Contributor Author
@brcrista brcrista Dec 27, 2019

Choose a reason for hiding this comment

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

SemVer stylizes its name as such. See https://semver.org/#semantic-versioning-specification-semver

@@ -1,7 +1,7 @@
extends: default

rules:
# 80 chars should be enough, but don't fail if a line is longer
# 120 chars should be enough, but don't fail if a line is longer
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had tried spreading the description, etc., across multiple lines, but I decided it would probably be better to raise the limit. Monitors have gotten bigger since the days of the 80-char guideline, so I think 120 is a good number today.

@brcrista
Copy link
Contributor Author

@konradpabjan I just noticed we don't run the main workflow on pull requests. Do you want me to enable it?

Copy link
Collaborator
@konradpabjan konradpabjan left a comment

Choose a reason for hiding this comment

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

LGTM!

@brcrista
Copy link
Contributor Author

@konradpabjan can you merge? I don't have write access to this repo.

@konradpabjan konradpabjan merged commit f907029 into actions:master Dec 30, 2019
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.

2 participants
0