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

Simplify and Groovyfy attachment handling #536

Merged
merged 5 commits into from
Mar 15, 2019
Merged

Conversation

mguillem
Copy link
Contributor

@mguillem mguillem commented Mar 6, 2019

Finally the complete pull request to handle attachments "in a Groovy way".

The original idea was here: #392 but it was still requiring an import that had to be whitelisted. This is not the case anymore and this change is particularly interesting as it removes the need for whitelisting JSONArray and JSONObject when using attachments.

Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

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

Thanks, I did try get this to work awhile ago but had issues passing an object in.
Nice work, code looks good, I'll merge if manual testing doesn't show any issues

Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

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

Functionality is great, thanks for doing this!
Can you revert the import ordering changes please
SlackSendStepTest and SlackSendStep

@mguillem
Copy link
Contributor Author

import order changed (without explicit rules for the expected order it is just a requirement to demotivate contributors :-()

@timja
Copy link
Member

timja commented Mar 15, 2019

Without explicit rules the default is always minimal change, which may mean deactivating auto import in the project you are in.

I will look at enforcing it with checkstyle soon

@jetersen
Copy link
Member

jetersen commented Mar 15, 2019

@timja codacy has support for reading checkstyle rules but you can also setup Jenkinsci to do it, but Jenkins CI is a bit slower.

Code climate is another option

@jetersen
Copy link
Member

on JCasC I would properly switch to code climate instead of codacy, https://docs.codeclimate.com/docs/checkstyle better documentation and better config

@timja
Copy link
Member

timja commented Mar 15, 2019

@Casz checkstyle is already run as part of the build, there's just minimal rules.
I have a local branch but never finished cleaning up the codebase for it.

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