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

Adds "Cancel" button to UI #5003

Merged

Conversation

@jessemarple
Copy link
Contributor

commented Jun 27, 2019

Adds a Cancel button to the PluginConfig UI in Project Nodes Config.

@jessemarple jessemarple requested a review from gschueler Jun 27, 2019

@gschueler
Copy link
Member

left a comment

can you separate the functional changes, from the non-functional changes into 2 separate commits? i.e. put the reformatting changes in a separate commit. I can't tell what has been changed

@jessemarple

This comment has been minimized.

Copy link
Contributor Author

commented Jun 28, 2019

@gschueler I can't check in functional from formatting changes. The formatting changes were not made intentionally, but instead with VS Code as it maintains the best practice formatting automatically. I'll get you set up with a formatter such that when you save a file it automatically formats.

@jessemarple jessemarple merged commit 35c2eb9 into master Jun 28, 2019

20 checks passed

Mergeable Mergeable Run has been Completed!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
security/snyk - build.gradle (rundeck) No manifest changes detected
security/snyk - core/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/copyfile-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/flow-control-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/git-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/jasypt-encryption-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/job-state-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/localexec-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/orchestrator-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/script-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/source-refresh-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/stub-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/upvar-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - rundeck-storage/build.gradle (rundeck) No manifest changes detected
security/snyk - rundeckapp/build.gradle (rundeck) No manifest changes detected
security/snyk - rundeckapp/grails-spa/package.json (rundeck) No manifest changes detected
security/snyk - rundeckapp/metricsweb/build.gradle (rundeck) No manifest changes detected
@jessemarple
Copy link
Contributor Author

left a comment

Review complete.

@gschueler

This comment has been minimized.

Copy link
Member

commented Jun 30, 2019

@jessemarple the reformatting is not the primary problem, it is the combining the of reformatting with other changes in the same commit. I've written up my problems with this kind of change here https://github.com/rundeck/rundeck/wiki/Github-Guidelines#reformatting-code-and-non-functional-changes please try to separate the changes in the future. (If you have to reformat the file, commit that reformatted file before making changes to it.)

@jessemarple

This comment has been minimized.

Copy link
Contributor Author

commented Jul 1, 2019

@gschueler I totally understand how problematic this is. The issue we're having isn't that the reformatting changed the code, it's that when the code was originally written, it was formatted incorrectly or not formated at all. Using the formats as defined in the grails-spa .eslintrc.js and .editorconfig files would eliminate the issue. The reason those files are included in the repo is for exactly that reason - to standardize the formatting of Vue and JS files across different editors and developers.

I'll make an update to the Wiki page to include that requirement.

@jessemarple

This comment has been minimized.

Copy link
Contributor Author

commented Jul 1, 2019

I've updated the Github Guildlines wiki to include information regarding the StandardJS ruleset and how to use it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.