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

Fixes validation of .workflow #529

Merged

Conversation

JayjeetAtGithub
Copy link
Collaborator

Adds validation rules for .workflow files

@pep8speaks
Copy link

pep8speaks commented Mar 6, 2019

Hello @JayjeetAtGithub! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-03-15 03:11:20 UTC

ci/test/validate1 Outdated Show resolved Hide resolved
ci/test/validate2 Outdated Show resolved Hide resolved
cli/popper/gha.py Outdated Show resolved Hide resolved
cli/popper/gha.py Outdated Show resolved Hide resolved
cli/popper/gha.py Outdated Show resolved Hide resolved
cli/popper/gha.py Outdated Show resolved Hide resolved
cli/popper/gha.py Outdated Show resolved Hide resolved
cli/popper/gha.py Outdated Show resolved Hide resolved
@JayjeetAtGithub
Copy link
Collaborator Author

JayjeetAtGithub commented Mar 7, 2019

@ivotron Can you please tell me what logic will be in the validate function if the validation is handled in normalize function ? As of now , before normalizing , all the validation errors are being catched in validate() and only if everthing is valid , we move on to normalizing in normalize()

@JayjeetAtGithub JayjeetAtGithub force-pushed the jayjeet/workflow-validation-fixes branch 2 times, most recently from a38a384 to 0f53d6d Compare March 7, 2019 14:30
@ivotron
Copy link
Collaborator

ivotron commented Mar 8, 2019

hey @JayjeetAtGithub

As of now , before normalizing , all the validation errors are being catched in validate() and only if everthing is valid , we move on to normalizing in normalize()

you're right. What I meant in my review was that, we wouldn't be doing any type-checking as part of the validate() method that you're implementing. Instead we would do type-checking in the normalize function. Maybe we can rename it to normalize_types() or something like that. And your method could be validate_syntax or parse?

@ivotron
Copy link
Collaborator

ivotron commented Mar 12, 2019

hey @JayjeetAtGithub . Is there anything you would like to discuss regarding this? Thanks!

@JayjeetAtGithub
Copy link
Collaborator Author

@ivotron Can you please tell what will be in there in validate_syntax() , since what my pr adds is only type checking , and if i do that in normalize_types() , then i cant understand what validate_syntax() / parse() will do ? Do i need to add some other logic ?

@ivotron
Copy link
Collaborator

ivotron commented Mar 13, 2019

@JayjeetAtGithub validate_syntax will only check for the presence of required elements, as specified in #504 . The normalize_types will look at types

@JayjeetAtGithub JayjeetAtGithub force-pushed the jayjeet/workflow-validation-fixes branch from daf829f to 04aaf84 Compare March 13, 2019 11:35
@ivotron
Copy link
Collaborator

ivotron commented Mar 15, 2019

Thanks a lot @JayjeetAtGithub ! There are a couple of pep8 issues that pep8speaks is complaining about. Also, could you please rebase master again? Sorry 😩

@JayjeetAtGithub JayjeetAtGithub force-pushed the jayjeet/workflow-validation-fixes branch from e544023 to e751006 Compare March 15, 2019 02:50
@JayjeetAtGithub JayjeetAtGithub force-pushed the jayjeet/workflow-validation-fixes branch from e751006 to d159905 Compare March 15, 2019 02:58
@ivotron ivotron merged commit ab14a6a into getpopper:master Mar 15, 2019
@ivotron
Copy link
Collaborator

ivotron commented Mar 15, 2019

thanks a lot for your patience @JayjeetAtGithub !

ivotron pushed a commit that referenced this pull request May 24, 2020
Adds validation for workflow files.

fixes #504
ivotron pushed a commit that referenced this pull request May 25, 2020
Adds validation for workflow files.

fixes #504
ivotron pushed a commit that referenced this pull request May 25, 2020
Adds validation for workflow files.

fixes #504
ivotron pushed a commit that referenced this pull request May 25, 2020
Adds validation for workflow files.

fixes #504
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