APEXMALHAR-2461 fix dependencies with license in category x

[ananthc]

@vrozov / @tweise : Could anyone of you please review and merge ?

[vrozov]

LGTM

[tweise]

Did you verify that the dependencies that go into the .apa remain the same?

[pramodin]

Were you able to verify any of the operators/examples with the new dependency?

[vrozov]

Did you verify that the dependencies that go into the .apa remain the same?

@tweise Thomas, please clarify. I will expect the new version of the dependencies to be packaged into the .apa affected by the change. Using 'optional' should not affect .apa packaging as it was already used with affected dependency in few examples.

Were you able to verify any of the operators/examples with the new dependency?

@PramodSSImmaneni Pramod, it is not clear what verification is expected in addition to an operator provided unit tests.

[pramodin]

examples sometimes don't have proper unit tests

[vrozov]

It is an issue of examples and a committer who accepted them without proper unit tests, not of this PR.

[pramodin]

I am not sure how that helps this situation but if the unit tests are not appropriate, a quick sanity test of one or two examples is sufficient to move forward with the PR.

[tweise]

@vrozov yes, I want to confirm that the dependency that was marked optional is still included in the .apa.

I also agree that no other manual testing needs to be performed for this change.

[pramodin]

I would strongly suggest that we perform some sanity checks instead of blindly doing a version upgrade if the unit tests are falling short.

[ananthc]

I am not sure how that helps this situation but if the unit tests are not appropriate, a quick sanity test of one or two examples is sufficient to move forward with the PR.

@PramodSSImmaneni - Since we have tests written for the end to end example application itself and these are triggered during the maven test phase, I am confident that the sanity checks are being taken care of. For example, the JSON schema validator is being effected in the examples/parser application as an end to end test and it is passing the checks. For the changes related to mysql connector version changes, the main impacts are solely on the twitter example application itself and there are no examples that use twitter example as a dependency, I think this is also taken care of.

In fact, all the changes to examples are not really impacting much in terms of dependencies as the examples are being pushed as "apa" artefacts to the repository and not as "jar" artefacts.

[ananthc]

Did you verify that the dependencies that go into the .apa remain the same?

@tweise - I just verified and they look good to me. Unpacked the twitter apps and the enricher apps and they have the right versions as expected.

[vrozov]

I will not block a contribution with more unit testing, but it is not reasonable to expect more sanity checks as part of this PR. If somebody wants to pick up this task and do more sanity checks, the contribution is welcome.

The only way to help with the current situation is to ensure that contributors include unit test with their contribution. It is already part of the Apex contribution guidelines.

Unless there is a technical reason (not a hypothetical possibility of broken functionality) I'll merge the PR in a day to unblock 3.8 release.

[tweise]

In fact, all the changes to examples are not really impacting much in terms of dependencies as the examples are being pushed as "apa" artefacts to the repository and not as "jar" artefacts.

Examples are not released as Maven artifacts at all. LGTM

[pramodin]

@ananthc that sounds good. I was looking for some sort of minimum coverage between sanity checks and tests, from your explanation it looks like you have done sufficient verification. LGTM