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

Website updates #5

Merged
merged 8 commits into from
Jul 7, 2024
Merged

Website updates #5

merged 8 commits into from
Jul 7, 2024

Conversation

lucedes27
Copy link
Contributor

  • Create CFLogo.png
  • Update API folder
  • Introduce GitHub workflow to copy changes to gh-pages branch

- Create CFLogo.png
- Update API folder
- Introduce GitHub workflow to copy changes to gh-pages branch
Copy link
Member

@wmdietl wmdietl left a comment

Choose a reason for hiding this comment

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

Thanks for these fixes! I have a few questions about them.

@@ -161,6 +161,9 @@ public static void main(String[] args) throws IOException {
e.printStackTrace();
}

File releaseOrg = new File(String.valueOf(javadocFolder) + "/checker-javadoc/org");
FileUtils.copyDirectoryToDirectory(releaseOrg, javadocFolder);
Copy link
Member

Choose a reason for hiding this comment

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

Isn't it enough to move the directory over? Or why do we need two copies of the javadoc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we could just move the directory I believe

@@ -305,6 +308,11 @@ public static void main(String[] args) throws IOException {

System.out.println("Latest release: " + String.valueOf(latestRelease));

// Copy CFLogo.png to cf/
File cfLogo = new File(String.valueOf(latestRelease) + "/tutorial/CFLogo.png");
Copy link
Member

Choose a reason for hiding this comment

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

Why does this PR need to add file CFLogo.png? Doesn't this take the existing logo from the latest release?
Or is that logo needed at https://eisop.github.io/?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I had initially wanted to upload the .png file, but decided it was better to take it from the latest release in case you want to change the logo at some point.
Will remove the file.

@@ -34,6 +34,14 @@ jobs:
- name: Build with Maven
run: mvn -B package --file pom.xml

# Copy the files to the gh-pages branch
Copy link
Member

Choose a reason for hiding this comment

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

Should this really be executed for a PR? It would seem odd that each PR run updates what is displayed on the live website.
Also, as EisopSiteGenerator isn't executed, the templates and releases aren't updated.
So maybe this should go somewhere else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure. The alternative is having to copy over the changes from the main branch to the gh-pages branch manually. This just saves a step before running EisopSiteGenerator on the gh-pages branch.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks! From the log it looks like this isn't working correctly. I've filed #11 for follow-up work.

@@ -305,6 +308,16 @@ public static void main(String[] args) throws IOException {

System.out.println("Latest release: " + String.valueOf(latestRelease));

// Rename cf/manual/manual.pdf to cf/manual/checker-framework-manual.pdf
File manualPDF = new File(String.valueOf(directoryPath) + "/manual/manual.pdf");
Copy link

Choose a reason for hiding this comment

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

9% of developers fix this issue

PATH_TRAVERSAL_IN: This API (java/io/File.(Ljava/lang/String;)V) reads a file whose location might be specified by user input


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.

@@ -305,6 +308,16 @@ public static void main(String[] args) throws IOException {

System.out.println("Latest release: " + String.valueOf(latestRelease));

// Rename cf/manual/manual.pdf to cf/manual/checker-framework-manual.pdf
File manualPDF = new File(String.valueOf(directoryPath) + "/manual/manual.pdf");
File checkerFrameworkManualPDF = new File(String.valueOf(directoryPath) + "/manual/checker-framework-manual.pdf");
Copy link

Choose a reason for hiding this comment

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

9% of developers fix this issue

PATH_TRAVERSAL_IN: This API (java/io/File.(Ljava/lang/String;)V) reads a file whose location might be specified by user input


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.

- Remove useless CFLogo.png
- Avoid having two copies of checker-javadoc/org folder
@wmdietl wmdietl merged commit 40c0aec into eisop:master Jul 7, 2024
1 check passed
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.

2 participants