jakartaee / batch

The Jakarta Batch project produces the Batch Specification and API.
https://projects.eclipse.org/projects/ee4j.batch
Apache License 2.0
13 stars 17 forks source link

Adust language around "dynamic" batch property access per TCK challenge issue #71 #211

Closed scottkurz closed 3 months ago

scottkurz commented 3 months ago

Fixes #209. (Also use this as an opportunity to update the repo URLs since we moved to the 'jakartaee' GitHub org)

For your review: @Azquelt @rmannibucau @rzo1 (you can ignore the link rename and just look at the 2nd commit)

Response to Suggestions

@rzo1, I did read your suggestions: https://github.com/jakartaee/batch/issues/209#issuecomment-1969807221 for related updates.

So, we do have the special case https://github.com/jakartaee/batch/blob/master/spec/src/main/asciidoc/batch_programming_model.adoc#method-parameter-and-constructor-parameter-injection-with-explicit-name:

A key difference vs. field injection, however, is that method and constructor parameter injection are not required to support the default batch property name like it is calculated from the field name in field injection. A method or constructor parameter @BatchProperty annotation must explicitly include a 'name' attribute specifying the batch property name.

This is maybe a bit unusual.. and maybe is a legacy of the original spec's stance that it wouldn't require CDI (though we've more recently decided to embrace CDI).

But I feel like we don't especially need to go any further here.

If we do there is a decent chance we'll miss some other place with similar language where we should've similarly updated.
Plus we end up just re-documenting CDI itself the further we go.

What's there has already been approved...presumably what's there is more helpful than confusing at the moment...except for this small piece I'm changing.

Thanks though, and please see what you think.

rzo1 commented 3 months ago

@struberg guess your feedback would also be appreciated here.

scottkurz commented 3 months ago

Having gotten direction from the spec committee, I am planning to close out (and merge) this PR soon.

I've updated the PR to only include two things:

I appreciate the other comments from @rmannibucau and @rzo1. I know there were quite a few ideas and variations mentioned and suggested.

I don't have a clear sense of where to go with those, however, so will suggest they be included in a later issue/PR/discussion if desired. Thanks.