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

(PDK-1090) Test task names #18

Merged
merged 1 commit into from
Jul 23, 2018
Merged

(PDK-1090) Test task names #18

merged 1 commit into from
Jul 23, 2018

Conversation

jarretlavallee
Copy link
Contributor

Prior to this PR, the task names were not validated. This commit
updates the creates a rake task to check that the puppet task names
are compliant with the naming rules. Note that this is a workaround to
PDK-1090 and does not affect pdk validate. To use it run bundle exec rake tasknames.

The naming rules are in https://puppet.com/docs/bolt/0.x/writing_tasks.html#concept-676 and the files are tested against the \A[a-z][a-z0-9_]*\.[a-z0-9_]+\Z regex. I am also checking the file extension to be lowercase as well.

Example output:

$ bundle exec rake tasknames
Task name "tasks/123.json" is invalid
Task name "tasks/kb1234" is invalid
Task name "tasks/kb123_Sometask.sh" is invalid
Task name "tasks/kb1234-something.sh" is invalid

Copy link
Collaborator

@MartyEwings MartyEwings left a comment

Choose a reason for hiding this comment

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

Brilliant! Looks great

.travis.yml Outdated
- /.*/
- master
- /^v\d/
- /.*/
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like some extra whitespace snuck in here.

.travis.yml Outdated
@@ -13,19 +13,19 @@ script:
- 'bundle exec rake $CHECK'
bundler_args: --without system_tests
rvm:
- 2.4.1
- 2.4.4
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this commit require a ruby version bump?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does not. This is just a remnant of my local PDK being newer than 1.6.0. 1.6.1 will use 2.4.4 for testing.

metadata.json Outdated
@@ -91,7 +91,7 @@
"version_requirement": ">= 4.7.0 < 6.0.0"
}
],
"pdk-version": "1.6.0",
"pdk-version": "1.6.1.pre (heads/master-0-g9a9b458)",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Similar to above re: ruby, does this commit require a pdk-version bump? I honestly don't know.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope.

@jarretlavallee
Copy link
Contributor Author

@abottchen should I revert this to the pdk-templates at 1.6.0? The only manual changes in this commit are the .sync.yaml as that is the file used to generate the other files within the commit with pdk updated. Since I originally updated this with the head of pdk it used the most recent version of the pdk-templates at that time. So if I revert to 1.6.0 release it will be a much bigger commit due to the number of changes in the template.

Maybe the next best thing would be to waiting for 1.6.1 to release an update to the release version within this PR. Thoughts?

@abottchen
Copy link
Collaborator

@jarretlavallee There is probably a really cool git way to do it right, but the way I'd do it is just manually fix those specific lines in your local copy and push a new commit of this PR. Remove the extra whitespace from .travis.yml and set the version to 2.4.1, then set the pdk-version to 1.6.0 in metadata.json. Commit and push, and the PR should be clean.

Prior to this commit the task names were not validated. This commit
updates the creates a rake task to check that the puppet task names
are compliant with the naming rules. *Note* that this is a workaround to
PDK-1090 and does not affect `pdk validate`. To use it run `bundle exec
rake tasknames`.
@jarretlavallee
Copy link
Contributor Author

@abottchen reverted the files and rebased/squashed. I'll put up a PR with 1.6.1 when it releases.

Copy link
Collaborator

@abottchen abottchen left a comment

Choose a reason for hiding this comment

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

TGTM

@MartyEwings MartyEwings merged commit 8d1cf58 into puppetlabs:master Jul 23, 2018
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.

3 participants