add getConverter method to Config

[emattheis]

as proposed in #492

[Emily-Jiang]

I disagree with the approach. Let's discuss more on the issue.

[emattheis]

@Emily-Jiang in light of #514 can we move forward with this? I think we're in agreement that an external converter spec has merit, and Config can adopt it in the future, but having access to the converters within the context of a Config is useful regardless of what spec they follow.

[Emily-Jiang]

@emattheis please see my explaination on the issue.

[eclipse-microprofile-bot]

Can one of the admins verify this patch?

[dmlloyd]

@emattheis would you mind rebasing this? In particular I think the OSGi version thing probably can be dropped (we should update that separately for 2.0 anyway).

[radcortez]

.

[dmlloyd]

@emattheis not to continue to harp on you :) but would you mind reducing this down to a single commit which adds the new method? I don't think you need to update the release notes per se (at least I don't think we've discussed that as a group, that I recall anyway). But it shouldn't be 8 commits.

Alternatively you might want to check the "[ ] Allow edits from maintainers" box, and I can do it, if you don't have availability.

Thanks!

[emattheis]

@dmlloyd I meant to ask about that in the hangout. In my day-job, we use the "[] Allow squash merging" setting so we can do that when we merge PRs into the mainline. That way we preserve individual commits in the feature branches. Not sure if that's enabled on this repo, though.

Allow edits from maintainers is enabled, so feel free to squash on this branch if you prefer.

[dmlloyd]

All fixed up. @Emily-Jiang can you please re-review or dismiss your review?

[dmlloyd]

This needs a rebase after #544 is merged in order to pass CI.

[radcortez]

Do we have "Require branches to be up to date before merging" on?

[dmlloyd]

I don't think so. Might be a good idea though?

CI is failing because of #544 but that's an easy fix (Erik already did it for us in fact).

[radcortez]

Yes, I advice to turn it on. I can't do it.

[dmlloyd]

I think it would need some discussion (but anyway I don't have permission either). Open a process change issue?

[radcortez]

Sure.

[emattheis]

I updated the TCK with coverage for getConverter and used a local build of smallrye-config to verify. I'll squash the commits again once someone takes a look.

[radcortez]

We do need #544 to be merged so we can pass the CI build.

[dmlloyd]

It looks as though CI is presently testing with the branch head rather than a merge of the commit. Looking into it.

[emattheis]

want me to just rebase now that #544 is merged?

[dmlloyd]

I'm going to try to fix the action first. If that fails, then yes we'll need to rebase all of these.

[dmlloyd]

Actually it seems that the settings from the branch are used for Actions, so even if I fix CI upstream the branch will still break. So you'll still have to rebase, but let's still wait until #547 goes in, just so we can get the extra validation for it.

[dmlloyd]

@emattheis OK please go ahead and rebase! Thanks.

[dmlloyd]

@Emily-Jiang this one is also green now; can you please re-review or dismiss your old review?

[Emily-Jiang]

LGTM! Thanks @emattheis !