Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build(gradle): javadoc enhance + namings migration #473

Merged
merged 15 commits into from Dec 15, 2021

Conversation

stachu540
Copy link
Contributor

@stachu540 stachu540 commented Dec 3, 2021

Prerequisites for Code Changes

  • This pull request follows the code style of the project
  • I have tested this feature

Changes Proposed

  • Package Groups - see this example
  • aggregateJavadoc task behavior changes
    • removes directory while activating task
    • clones element-list as package-list in the same directory
    • task is active if Gradle using JVM9+
  • adding some links which points to specific class documentation. (some of them doesn't have a package-list file attached into index of javadoc)
  • small fix javadoc when it should not be a tag which recognize as unknown while compile javadoc. Adds code block in this fix.
  • adding overview files
  • Javadoc generates into en localization - no matter which localization is on your PC
  • obsolete unnecessary artifactId from buildSrc
  • migrate module names into settings.gradle.kts

Additional Information

This is proposal changes to javadoc, which adds Groups into specific packages, changes to javadoc aggregation and adding some links to the other javadoc documentation. Got adding some improvments with Javadoc Overview, constant localization and module names migrated to settings.gradle.kts. Each changes are tested. Only left is some typings, some improvments and it is ready to review.

i have adding some code block in those point
`base.archivesName` is a `Property<String>`. package names have been unchanged
removes directory while activating task `aggregateJavadoc`
clone `element-list` to `package-list` in the same directory
adding links - points to specific class documentation. (some of them doesn't have a `package-list` file attached into index of javadoc)
@stachu540 stachu540 marked this pull request as ready for review December 3, 2021 14:10
@stachu540 stachu540 marked this pull request as draft December 5, 2021 00:33
@stachu540 stachu540 marked this pull request as ready for review December 5, 2021 05:48
@PhilippHeuer
Copy link
Member

PhilippHeuer commented Dec 7, 2021

i get the following 2 errors when running aggregateJavadoc, javadoc works fine. It seems like aggregateJavadoc is more strict on validation.

...\twitch4j\buildSrc\overview-general.html:59: error: no summary or caption for table
</table>
^
...\twitch4j\buildSrc\overview-general.html:60: error: heading used out of sequence: <H3>, compared to previous heading: <H1>
<h3>To getting started please refer to our documentation</h3>

I think we need to replace the h3 with h2 in both html templates and add a <caption>Heading</caption> element inside of the package name / group name table.

- paragraph moved to caption table
- img is self-closed in html5
- fix sponsor heading
- replace heading in overview-single.html
@stachu540
Copy link
Contributor Author

got made some tweaks in requested files

@stachu540 stachu540 changed the title build(gradle): javadoc enhancement build(gradle): javadoc enhancement + namings migration Dec 7, 2021
@stachu540 stachu540 changed the title build(gradle): javadoc enhancement + namings migration build(gradle): javadoc enhance + namings migration Dec 7, 2021
Copy link
Member

@PhilippHeuer PhilippHeuer left a comment

Choose a reason for hiding this comment

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

I made a few more changes - the name of one module was wrong and i simplified the templates a little because they took up to much space - i think a few useful links are enough, most information should be in the documentation.

Copy link
Contributor Author

@stachu540 stachu540 left a comment

Choose a reason for hiding this comment

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

One last take before we getting into it.

buildSrc/overview-general.html Outdated Show resolved Hide resolved
buildSrc/overview-general.html Outdated Show resolved Hide resolved
buildSrc/overview-single.html Outdated Show resolved Hide resolved
PhilippHeuer and others added 3 commits December 9, 2021 00:14
Co-authored-by: Damian Staszewski <359222+stachu540@users.noreply.github.com>
Co-authored-by: Damian Staszewski <359222+stachu540@users.noreply.github.com>
@stachu540
Copy link
Contributor Author

LGTM

Looks like we can push this one.

@PhilippHeuer PhilippHeuer merged commit c57166f into twitch4j:develop Dec 15, 2021
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.

None yet

2 participants