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

docs: Overhaul javadocs, using Oracle's styleguide #264

Merged
merged 1 commit into from
Mar 4, 2020

Conversation

jamierocks
Copy link

I do not know what your stance on work-in-progress pull requests is - apologies if its don't make them. However, I believe that this pr could well do with community input throughout its development, to ensure that the javadocs are well made - hence opening the pr earlier that I normally would.

This pr should address #130.


This pull request aims to accomplish two objectives:

  1. Provide a strong set of javadocs, that complement the method name rather than regurgitate it.
  2. To fully utilize the capabilities of javadocs, to ensure once compiled they can aid developers in using sendgrid-java.

See http://www.oracle.com/technetwork/java/javase/documentation/index-137868.html#styleguide

@thinkingserious thinkingserious added the status: code review request requesting a community code review or review from Twilio label Oct 15, 2017
@SendGridDX
Copy link
Collaborator

SendGridDX commented Oct 15, 2017

CLA assistant check
All committers have signed the CLA.

@thinkingserious thinkingserious added difficulty: medium fix is medium in difficulty hacktoberfest status: work in progress Twilio or the community is in the process of implementing labels Oct 16, 2017
@thinkingserious
Copy link
Contributor

Hi @jamierocks,

Thank you! No problem at all with WIP :)

With Best Regards,

Elmer

@jamierocks
Copy link
Author

Hi @thinkingserious,

Glad to hear that ;) I was wondering if you have an IRC channel, or another way of real-time communication - I would quite like to discuss some other potential pull requests with you.

Regards,
Jamie

@jamierocks jamierocks changed the title [WIP] Overhaul javadocs, using Oracle's styleguide Overhaul javadocs, using Oracle's styleguide Oct 16, 2017
@thinkingserious thinkingserious added the status: code review request requesting a community code review or review from Twilio label Oct 16, 2017
@thinkingserious
Copy link
Contributor

thinkingserious commented Oct 17, 2017

Hi @jamierocks,

Could you please shoot us an email at dx@sendgrid.com so we can add you to a Slack channel we use?

With Best Regards,

Elmer

@jamierocks
Copy link
Author

Hi @thinkingserious,

I sent an email your way and still haven't got any invite to the Slack channel :(
Concerning the pr, whats the procedure like for merging at sendgrid? (is there anything I need to do? wait for feedback, etc)

Regards,
Jamie

@mbernier
Copy link
Contributor

mbernier commented Nov 1, 2017

We are just really behind on all the things - it's been a crazy month!

Since you have been looking at javadoc - I found this in another build and I'm curious if this is related to this PR?

:javadoc/home/travis/build/sendgrid/sendgrid-java/src/main/java/com/sendgrid/helpers/mail/objects/Attachments.java:205: warning: no @return
    public Builder withType(String type) {
                   ^
/home/travis/build/sendgrid/sendgrid-java/src/main/java/com/sendgrid/helpers/mail/objects/Attachments.java:214: warning: no @return
    public Builder withDisposition(String disposition) {
                   ^
/home/travis/build/sendgrid/sendgrid-java/src/main/java/com/sendgrid/helpers/mail/objects/Attachments.java:223: warning: no @return
    public Builder withContentId(String contentId) {
                   ^
/home/travis/build/sendgrid/sendgrid-java/src/main/java/com/sendgrid/helpers/mail/objects/Attachments.java:231: warning: no @return
    public Attachments build() {
                       ^
/home/travis/build/sendgrid/sendgrid-java/src/main/java/com/sendgrid/SendGridAPI.java:18: error: @param name not found
   * @param apiKey is your SendGrid API Key: https://app.sendgrid.com/settings/api_keys
            ^
1 error
4 warnings
:javadoc FAILED
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':javadoc'.
> Javadoc generation failed.
* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output.
BUILD FAILED
Total time: 18.18 secs
The command "eval ./gradlew assemble " failed. Retrying, 2 of 3.
The TaskContainer.add() method has been deprecated and is scheduled to be removed in Gradle 2.0. Please use the create() method instead.
:compileJava UP-TO-DATE
:processResources UP-TO-DATE
:classes UP-TO-DATE
:fatJarPrepareFiles UP-TO-DATE
:fatJar UP-TO-DATE
:jar UP-TO-DATE
:javadoc/home/travis/build/sendgrid/sendgrid-java/src/main/java/com/sendgrid/helpers/mail/objects/Attachments.java:205: warning: no @return
    public Builder withType(String type) {
                   ^
/home/travis/build/sendgrid/sendgrid-java/src/main/java/com/sendgrid/helpers/mail/objects/Attachments.java:214: warning: no @return
    public Builder withDisposition(String disposition) {
                   ^
/home/travis/build/sendgrid/sendgrid-java/src/main/java/com/sendgrid/helpers/mail/objects/Attachments.java:223: warning: no @return
    public Builder withContentId(String contentId) {
                   ^
/home/travis/build/sendgrid/sendgrid-java/src/main/java/com/sendgrid/helpers/mail/objects/Attachments.java:231: warning: no @return
    public Attachments build() {
                       ^
/home/travis/build/sendgrid/sendgrid-java/src/main/java/com/sendgrid/SendGridAPI.java:18: error: @param name not found
   * @param apiKey is your SendGrid API Key: https://app.sendgrid.com/settings/api_keys
            ^
1 error
4 warnings
:javadoc FAILED
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':javadoc'.
> Javadoc generation failed.
* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output.
BUILD FAILED
Total time: 7.54 secs
The command "eval ./gradlew assemble " failed. Retrying, 3 of 3.
The TaskContainer.add() method has been deprecated and is scheduled to be removed in Gradle 2.0. Please use the create() method instead.
:compileJava UP-TO-DATE
:processResources UP-TO-DATE
:classes UP-TO-DATE
:fatJarPrepareFiles UP-TO-DATE
:fatJar UP-TO-DATE
:jar UP-TO-DATE
:javadoc/home/travis/build/sendgrid/sendgrid-java/src/main/java/com/sendgrid/helpers/mail/objects/Attachments.java:205: warning: no @return
    public Builder withType(String type) {
                   ^
/home/travis/build/sendgrid/sendgrid-java/src/main/java/com/sendgrid/helpers/mail/objects/Attachments.java:214: warning: no @return
    public Builder withDisposition(String disposition) {
                   ^
/home/travis/build/sendgrid/sendgrid-java/src/main/java/com/sendgrid/helpers/mail/objects/Attachments.java:223: warning: no @return
    public Builder withContentId(String contentId) {
                   ^
/home/travis/build/sendgrid/sendgrid-java/src/main/java/com/sendgrid/helpers/mail/objects/Attachments.java:231: warning: no @return
    public Attachments build() {
                       ^
/home/travis/build/sendgrid/sendgrid-java/src/main/java/com/sendgrid/SendGridAPI.java:18: error: @param name not found
   * @param apiKey is your SendGrid API Key: https://app.sendgrid.com/settings/api_keys
            ^
1 error
4 warnings
:javadoc FAILED
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':javadoc'.
> Javadoc generation failed.

@jamierocks
Copy link
Author

Fair enough - you've had an insane amount of prs to go through.

This pr builds with no javadoc issues, although there might be issues with recent upstream changes - I'll rebase the pr when I'm at my dev machine.

@thinkingserious
Copy link
Contributor

@jamierocks

We have not been able to merge your Pull Request, but because you are awesome - we wanted to make sure you could still get a SendGrid Hacktoberfest shirt.

Please go fill out our swag form before Nov 5th and we will send the shirt! (We know that you might have tried this before and it didn’t work, sorry about that!)

You have till Nov 5th to fill out this form in order to get the Hacktoberfest shirt!

Thank you for contributing during Hacktoberfest! We hope to see you in the repos soon! Just so you know, we always give away a SendGrid shirt for your first ever non-Hacktoberfest PR that gets merged.

@jamierocks
Copy link
Author

So this popped up on my mobile feed, this PR was ready to merge. I'm not sure whether this PR is now as comprehensive as it once was, or if it needs to be rebased again to be merged. I'll take a look when I get back :)

@thinkingserious
Copy link
Contributor

Thanks @jamierocks!

@thinkingserious thinkingserious added type: twilio enhancement feature request on Twilio's roadmap and removed status: work in progress Twilio or the community is in the process of implementing labels Oct 23, 2018
@jamierocks
Copy link
Author

Going over my open PRs, looks like this could be merged?

Copy link
Contributor

@childish-sambino childish-sambino left a comment

Choose a reason for hiding this comment

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

LGTM. Nice work.

@childish-sambino childish-sambino changed the title Overhaul javadocs, using Oracle's styleguide docs: Overhaul javadocs, using Oracle's styleguide Mar 4, 2020
@childish-sambino childish-sambino merged commit 90c27d1 into sendgrid:refactor Mar 4, 2020
@childish-sambino
Copy link
Contributor

@jamierocks Just realized this was based on the refactor branch. Was this intentional? If not, could you submit a new PR for adding these changes to master?

@jamierocks
Copy link
Author

@childish-sambino I'm going to guess at the time this was intentional, but clearly not now. That could explain why it still cleanly applies after a year though I guess.

I'll see the damage tomorrow - hopefully it should be a clean cherry-pick, see what javadoc warnings/errors are present, and make any final adjustments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: medium fix is medium in difficulty status: code review request requesting a community code review or review from Twilio type: twilio enhancement feature request on Twilio's roadmap
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants