Skip to content

Use newest enterprise-groovy version - #14

Closed
demus-nine wants to merge 1 commit into
virtualdogbert:masterfrom
demus-nine:master
Closed

Use newest enterprise-groovy version#14
demus-nine wants to merge 1 commit into
virtualdogbert:masterfrom
demus-nine:master

Conversation

@demus-nine

Copy link
Copy Markdown
Contributor

@virtualdogbert virtualdogbert left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I think that I need an example of how this will actually work, and why the change is necessary. This isn't to say what you're purposing isn't valid, I just want to understand, what this is and why you need it.

Also, the documentation would need to be updated to reflect the new configuration parameters allowedSuppressions and what those would do for the user.

I'm also thinking if/when I do let this in I'll probably bump the version, as this is new functionality and not just a bug fix...

@demus-nine

Copy link
Copy Markdown
Contributor Author

I've just been converting a project to compilestatic using the plugin. There are a couple of methods, that require dynamic overload resolution at runtime, because if you specifiy fx. Object as the parameter type it will always resolve to the overload with an object parameter. Codenarc also complains, so a @SuppressWarnings('NoDef') is added. Unfortunately the plugin is either on or off, so there is no way to exclude this one method from the def required. In most places def is the devil's work, but here it is not.

Fx:
void something(targetObject) { doSomethingWith(targetObject) }
where there is a varying number of doSomething overloads taking different types, and which I have no control over can only easily be achieved with runtime overload resolution.

In reality I would expect suppressions to be respected anyway.

@demus-nine

Copy link
Copy Markdown
Contributor Author

I've added a commit with the doc changes, but I can't get this PR to update and I can't create a new PR with the new commit. How do you update the PR with a new commit?

@demus-nine
demus-nine marked this pull request as draft June 29, 2020 13:36
@demus-nine demus-nine closed this Jun 29, 2020
@virtualdogbert

Copy link
Copy Markdown
Owner

Hmm strange you should be able to update a PR by just pushing new commits to the branch that you are trying to get merged in the PR... but created a new PR will work too, just lose the context of the old PR...

@apryamostanov

apryamostanov commented Jul 6, 2020

Copy link
Copy Markdown
Collaborator

Issue: #15
Plugin PR: #16
Linked plugin PR: #14
Correspondent library PR: virtualdogbert/enterprise-groovy#2

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