JanssenProject / jans

An open source enterprise digital identity platform for CIAM or workforce... Janssen is a distribution of standards-based, developer friendly, components that are engineered to work together in any cloud. #OAuth #OpenID #FIDO
https://docs.jans.io
Apache License 2.0
470 stars 74 forks source link

fix(docs): javadoc comments are inconsistent with code #4120

Open ossdhaval opened 1 year ago

ossdhaval commented 1 year ago

Note: This issue is marked as good first issue. You should take it up only if this is your initial contribution to the Janssen Project.

Describe the bug There are number of instances in the codebase where javadoc comments are not consistent with the actual code. For instance, here.

When we generate javadoc using mvn javadoc:javadoc, the build log shows errors like below.

[ERROR] /home/IdeaProjects/Janssen/jans/jans-auth-server/model/src/main/java/io/jans/as/model/crypto/signature/EDDSAKeyFactory.java:127: error: exception not thrown: java.security.NoSuchProviderException
[ERROR]      * @throws NoSuchProviderException
[ERROR]                ^
[ERROR] /home/IdeaProjects/Janssen/jans/jans-auth-server/model/src/main/java/io/jans/as/model/crypto/signature/EDDSAKeyFactory.java:128: error: exception not thrown: java.security.NoSuchAlgorithmException
[ERROR]      * @throws NoSuchAlgorithmException
[ERROR]                ^
[ERROR] /home/IdeaProjects/Janssen/jans/jans-auth-server/model/src/main/java/io/jans/as/model/crypto/signature/EDDSAPublicKey.java:36: error: @param name not found
[ERROR]      * @param publicKeyData

Regular builds don't fail due to errors like the above as modules have configured the javadoc plug-in in their pom.xml as below. As a result, the build goes through and generates the wrong javadocs.

<plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-javadoc-plugin</artifactId>
    <configuration>
        <failOnError>false</failOnError>
    </configuration>
</plugin>

To Reproduce Steps to reproduce the behavior:

  1. Find and comment out <failOnError>false</failOnError> configuration for javadoc plug-in across module pom files
  2. Now run mvn javadoc:javadoc for each module one-by-one
  3. Look for errors like the ones shown above in the build log.

Expected behavior javadoc comments should be updated to be consistent with the code

CapedBojji commented 1 year ago

im kinda new to this but will the error list give me the full list of methods with inconsistent javadocs?

ossdhaval commented 1 year ago

Hey @AFreeBaldy , thanks for checking this issue.

Yes. The Maven build log gives a complete list with errors pointing to the file name and line number. You might see other warnings too, but we want to focus on errors due to inconsistent javadocs.

CapedBojji commented 1 year ago

Hey @AFreeBaldy , thanks for checking this issue.

Yes. The Maven build log gives a complete list with errors pointing to the file name and line number. You might see other warnings too, but we want to focus on errors due to inconsistent javadocs.

I will get on it then

ossdhaval commented 1 year ago

Also, make sure to go through contribution guidelines to understand the process and overall standards.

ossdhaval commented 1 year ago

@AFreeBaldy I am assigning this to you. Let us know if you need any help.

CapedBojji commented 1 year ago

@AFreeBaldy I am assigning this to you. Let us know if you need any help.

I have a question, for cases where there are no params, what should I use as the description for the params tag. I am not 100% familiar with the code base, so If I don't use some general description, I will have to first familiarize myself with the code, then write an accurate description, which will take a while.

ossdhaval commented 1 year ago

Hey @AFreeBaldy, Can you please share a link to the code where you are seeing such a problem ?

I will have to first familiarize myself with the code, then write an accurate description

Yes, writing an accurate description would be essential, because an inaccurate description may hurt us more than not having documentation. Learning the code would be an investment if you intend to get more involved in the project. And we would love to see that. :heart_decoration:

We always ask contributors to create small PRs. With a small number of changes in each. It is easier to review and less risky to merge. Hence it moves faster. What you can do is you can keep all the problem cases in a separate PR and submit that at the last. Start with what is easy and obvious for you. Create a PR for that and submit it for review. This way, you'll get familiarized with the process also.

Hope this helps.

CapedBojji commented 1 year ago

Hey @AFreeBaldy, Can you please share a link to the code where you are seeing such a problem ?

I will have to first familiarize myself with the code, then write an accurate description

Yes, writing an accurate description would be essential, because an inaccurate description may hurt us more than not having documentation. Learning the code would be an investment if you intend to get more involved in the project. And we would love to see that. 💟

We always ask contributors to create small PRs. With a small number of changes in each. It is easier to review and less risky to merge. Hence it moves faster. What you can do is you can keep all the problem cases in a separate PR and submit that at the last. Start with what is easy and obvious for you. Create a PR for that and submit it for review. This way, you'll get familiarized with the process also.

Hope this helps.

ok thank you will do

ossdhaval commented 1 year ago

Hey @AFreeBaldy

This issue has been sitting idle for a couple of weeks. If you need any support, do let us know. If you are not planning to work on this issue, it will be better to keep this issue unassigned so that it becomes open for others in the community to pick up.

Please add a comment here. In absence of a comment, I would be removing the assignment tomorrow.

CapedBojji commented 1 year ago

Hey @AFreeBaldy

This issue has been sitting idle for a couple of weeks. If you need any support, do let us know. If you are not planning to work on this issue, it will be better to keep this issue unassigned so that it becomes open for others in the community to pick up.

Please add a comment here. In absence of a comment, I would be removing the assignment tomorrow.

I have a very important test for a program I am in coming up in a few weeks so I have been studying, but afterwards I plan on continuing, I was reading over the code base trying to familiarize myself with it. Sorry, but I can get started after my test