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

fix java search undefined hack #7574

Merged
merged 6 commits into from
Jun 1, 2022
Merged

fix java search undefined hack #7574

merged 6 commits into from
Jun 1, 2022

Conversation

dixler
Copy link
Contributor

@dixler dixler commented May 26, 2022

Proposed changes

Java SDK docs had broken search functionality. @cnunciato added a workaround to replace undefined in the url. This PR adds an undefined check and removes the workaround.

Unreleased product version (optional)

Related issues (optional)

fixes pulumi/pulumi-java#595

@github-actions
Copy link

Your site preview for commit ba83f0f is ready! 🎉

http://pulumi-docs-origin-pr-7574-ba83f0fc.s3-website.us-west-2.amazonaws.com.

Copy link

@pulumi pulumi bot left a comment

Choose a reason for hiding this comment

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

🍹 The Update (preview) for stack pulumi/www.pulumi.com/production was successful.

Resource Changes

    Name                  Type                                      Operation
~   cdn                   aws:cloudfront/distribution:Distribution  update
*   origin-bucket-policy  aws:s3/bucketPolicy:BucketPolicy          replaced

@dixler dixler changed the title wip fix java search fix java search undefined hack May 26, 2022
@dixler dixler marked this pull request as ready for review May 26, 2022 22:03
@github-actions
Copy link

Your site preview for commit 5b54495 is ready! 🎉

http://pulumi-docs-origin-pr-7574-5b544953.s3-website.us-west-2.amazonaws.com.

Copy link

@pulumi pulumi bot left a comment

Choose a reason for hiding this comment

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

🍹 The Update (preview) for stack pulumi/www.pulumi.com/production was successful.

Resource Changes

    Name                  Type                                      Operation
~   cdn                   aws:cloudfront/distribution:Distribution  update
*   origin-bucket-policy  aws:s3/bucketPolicy:BucketPolicy          replaced

@dixler dixler requested review from t0yv0 and zchase May 26, 2022 22:04
Copy link
Member

@t0yv0 t0yv0 left a comment

Choose a reason for hiding this comment

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

We perhaps should not be editing a file copyrighted Oracle. This folder is autogenerated by the Javadoc target - and the fix will be wiped when we regenerate. Can we find and fix the root cause? Thanks.

@dixler
Copy link
Contributor Author

dixler commented May 27, 2022

We perhaps should not be editing a file copyrighted Oracle. This folder is autogenerated by the Javadoc target - and the fix will be wiped when we regenerate. Can we find and fix the root cause? Thanks.

https://stackoverflow.com/questions/52326318/maven-javadoc-search-redirects-to-undefined-url

This bug should be fixed in JDK 12: https://bugs.openjdk.java.net/browse/JDK-8215291

Is bumping to JDK 12 fine?

@t0yv0
Copy link
Member

t0yv0 commented May 27, 2022

Interesting. It seems like changing default JDK is a big deal, more so than this error; we should at least retain some form of testing on 11. Can we scope the change to javadoc generation only? Generate Javadoc on 12 but the rest as-is on 11?

@dixler
Copy link
Contributor Author

dixler commented May 31, 2022

@t0yv0 sorry it took so long to reply. Was on escalation rotation and a p0 reopened itself.

Interesting. It seems like changing default JDK is a big deal, more so than this error; we should at least retain some form of testing on 11. Can we scope the change to javadoc generation only? Generate Javadoc on 12 but the rest as-is on 11?

I'm not sure. The two options are:

  1. Finding the root cause: It appears to be a known issue and I'd have to look into it which could take some time. I have a couple of other things regarding java docker to work on that I think has more interest.
  2. Patching: I can also make it so that we copy the patch when regenerating docs. I don't know if you'd prefer this patch or @cnunciato's, but whichever one you prefer, we can patch it in. It's GPL so afaik we can edit the file, but not the header.

@t0yv0
Copy link
Member

t0yv0 commented May 31, 2022

It's not possible to patch generated code by hand, as the patch will be removed next time we re-gen the docs using https://github.com/pulumi/docs/blob/master/scripts/gen_javadoc.sh . If you could include a script that auto-patches the output as part of code generation that could be a viable workaround for now.

@dixler
Copy link
Contributor Author

dixler commented May 31, 2022

@t0yv0 I've updated the script to overwrite the search.js file

@dixler dixler requested a review from t0yv0 May 31, 2022 17:19
@github-actions
Copy link

Your site preview for commit 0190f45 is ready! 🎉

http://pulumi-docs-origin-pr-7574-0190f457.s3-website.us-west-2.amazonaws.com.

Copy link

@pulumi pulumi bot left a comment

Choose a reason for hiding this comment

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

🍹 The Update (preview) for stack pulumi/www.pulumi.com/production was successful.

Resource Changes

    Name                  Type                                      Operation
~   cdn                   aws:cloudfront/distribution:Distribution  update
*   origin-bucket-policy  aws:s3/bucketPolicy:BucketPolicy          replaced

@t0yv0
Copy link
Member

t0yv0 commented Jun 1, 2022

Thank you! This makes it easy to keep updating the docs.

@dixler dixler merged commit 4051bc7 into master Jun 1, 2022
@pulumi-bot pulumi-bot deleted the dixler/595/fix-java-search branch June 1, 2022 20:46
@github-actions
Copy link

github-actions bot commented Jun 1, 2022

Site previews for this pull request have been removed. ✨

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.

Broken javadoc links
2 participants