jakartaee / interceptors

Jakarta Interceptors
https://eclipse.org/ee4j/interceptors
Other
11 stars 24 forks source link

Spec migration #66

Closed thadumi closed 4 years ago

thadumi commented 4 years ago

Hi @arjantijms, I prepared the new specification.

I have only one doubt regarding the paragraph "Default Interceptors". In JavaEE it was under chapter 4 (Associating Interceptors with Classes and Methods using the Interceptors Annotation) but into the raw spec it looks like it has been moved under chapter 2 (Interceptor Programming Contract). What do you think about it?

arjantijms commented 4 years ago

@thadumi The formatting and headers and chapters are mostly wrong in the raw document. You can correct them if you want. Thx!

thadumi commented 4 years ago

The formatting and headers and chapters are mostly wrong in the raw document.

I did another check with the JavaEE version and I've fixed some of them. Let me know if you notice something strange.

arjantijms commented 4 years ago

I did another check with the JavaEE version and I've fixed some of them. Let me know if you notice something strange.

The check is to just render the new spec docs, open the existing (Java EE) spec docs and compare them page by page. It should look as similar as possible, and the headings and chapters etc should all match. That's what I personally did with the security specs.

arjantijms commented 4 years ago

@thadumi I've requested review from the other committers. Let's wait until at least one has given feedback. Thanks a lot for your work! :)

bshannon commented 4 years ago

Please confirm that you're comparing against this Java EE version of the spec. If so, and if you preserve the organization and essential content, then you're probably good to go.

thadumi commented 4 years ago

Please confirm that you're comparing against this Java EE version of the spec.

Thanks for pointing out that document. I've been using another one. I'm going to do another check.

thadumi commented 4 years ago

@bshannon I just fixed all the chapters. Should be everything good now. Thanks for the hint.

thadumi commented 4 years ago

Did you split every section into its own file?

Exactly, I took the idea from jaxrs. Each section has it's own file and the folders are used for representing the hierarchical order. A file has the name of the section which contains inside and the folder has the name of the chapter/main section.

bshannon commented 4 years ago

I haven't looked at jaxrs but I think it's the outlier here. All the other ones that I have looked at have been dividing it up by chapter. I would expect stronger cohesion between the sections in a chapter than between the chapters in a book, so when dividing up the work is make sense to assign someone to work on a chapter. While it's true that then can then work on the 7 files that might make up the sections of that chapter, it might also seem like overkill to have the information for the chapter so spread out, and rearranging the content of a chapter to involve deleting and renaming files.

If no one else who's going to be working on these chapters has a strong opinion one way or another, we can leave it as is and see how it works out in practice. It's easy enough to suck all the content for a chapter into the file for a chapter if that's what someone wants.

arjantijms commented 4 years ago

Indeed, most projects have split their documents by chapter. Also in book writing in general the standard is to deliver files by chapter.

If it's not too much trouble it would be really great if the PR can be updated to organise per chapter.

arjantijms commented 4 years ago

@thadumi Maybe this one helps for inspiration: https://github.com/eclipse-ee4j/authorization/tree/master/spec/src/main/asciidoc/chapters

thadumi commented 4 years ago

I understand your point of view. I still think that for longer documents should be better splinting also the sections in single files.

Anyway, I merged all the sections within the respective chapters.

thadumi commented 4 years ago

Hi @arjantijms , can we merge this PR?

arjantijms commented 4 years ago

Hi @arjantijms , can we merge this PR?

yes, let's do it! Thanks a ton for all your work :)