Skip to content

Copy website images to data folder when flattening#93

Open
spatial-eli wants to merge 1 commit intoCCOMJHC:mainfrom
Spatialnetics:1-copy-website-images-to-data-folder-when-flattening
Open

Copy website images to data folder when flattening#93
spatial-eli wants to merge 1 commit intoCCOMJHC:mainfrom
Spatialnetics:1-copy-website-images-to-data-folder-when-flattening

Conversation

@spatial-eli
Copy link
Copy Markdown

No description provided.

@spatial-eli spatial-eli changed the title #1 Copy website images to data folder when flattening Copy website images to data folder when flattening Feb 25, 2026
@spatial-eli
Copy link
Copy Markdown
Author

@brian-r-calder, I don't have permission to request a reviewer. How does it normally work on your side? Do people tag you in comments?

@brian-r-calder
Copy link
Copy Markdown
Collaborator

@brian-r-calder, I don't have permission to request a reviewer. How does it normally work on your side? Do people tag you in comments?

To be honest, we haven't really had to deal with this before ... I'll check on the permissions, but in the meantime I'm adding myself and Brian Miles to this so that we get it reviewed and merged. I'm surprised: I thought we had done this already in the code. Thanks for the PR!

@brian-r-calder brian-r-calder added bug Something isn't working minor labels Mar 18, 2026
@spatial-eli
Copy link
Copy Markdown
Author

@brian-r-calder, I don't have permission to request a reviewer. How does it normally work on your side? Do people tag you in comments?

To be honest, we haven't really had to deal with this before ... I'll check on the permissions, but in the meantime I'm adding myself and Brian Miles to this so that we get it reviewed and merged. I'm surprised: I thought we had done this already in the code. Thanks for the PR!

Ok, so do you typically handle the merge yourself, then?

Also, are PRs a good place to suggest changes in the wiki, or do you have another procedure for that? In this case, it's quite minor but this line
"If it does not already exist, copy directory LoggerFirmware/website/images to LoggerFirmware/data/website/images. This provides the WIBL logo." in https://github.com/CCOMJHC/WIBL/wiki/Firmware#flashing-the-firmware-and-website could be removed.

@selimnairb selimnairb self-requested a review March 18, 2026 16:30
Copy link
Copy Markdown
Collaborator

@brian-r-calder brian-r-calder left a comment

Choose a reason for hiding this comment

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

I'm not entirely clear what the intent is for this PR.

We generally build the website for the firmware using the compile_website.sh script, which compiles down the JavaScript, uses the inline_flatten.py script to flatten the website into one-file HTML, and then makes the icon image and copies it and the other images into the website build directory (data/website).

The proposed PR only does part of this (i.e., copying the images into place). If the intent was to replace the compilation script, it needs to do all of the other things too; if the intent was just to package up images, then the compilation script already does that and this PR is redundant.

Could you please clarify the intent?

@brian-r-calder
Copy link
Copy Markdown
Collaborator

@brian-r-calder, I don't have permission to request a reviewer. How does it normally work on your side? Do people tag you in comments?

To be honest, we haven't really had to deal with this before ... I'll check on the permissions, but in the meantime I'm adding myself and Brian Miles to this so that we get it reviewed and merged. I'm surprised: I thought we had done this already in the code. Thanks for the PR!

Ok, so do you typically handle the merge yourself, then?

Also, are PRs a good place to suggest changes in the wiki, or do you have another procedure for that? In this case, it's quite minor but this line "If it does not already exist, copy directory LoggerFirmware/website/images to LoggerFirmware/data/website/images. This provides the WIBL logo." in https://github.com/CCOMJHC/WIBL/wiki/Firmware#flashing-the-firmware-and-website could be removed.

Yes, we're doing the merges ourselves for now (mostly because we've been the only ones contributing so far). That's something we could review in the future. The Wiki is a repository in itself, and can be cloned, but so far as I can tell there's no way to make formal PRs on it. Again, this is something that we haven't had to really tackle much before, so we're open to suggestions for how to do this well.

@spatial-eli
Copy link
Copy Markdown
Author

I'm not entirely clear what the intent is for this PR.

We generally build the website for the firmware using the compile_website.sh script, which compiles down the JavaScript, uses the inline_flatten.py script to flatten the website into one-file HTML, and then makes the icon image and copies it and the other images into the website build directory (data/website).

The proposed PR only does part of this (i.e., copying the images into place). If the intent was to replace the compilation script, it needs to do all of the other things too; if the intent was just to package up images, then the compilation script already does that and this PR is redundant.

Could you please clarify the intent?

I didn't use compile_website.sh, I followed the first steps in wiki that says to run inline_flatten.py, then manually copy the folder. I only just noticed that it does mention it can be achieved with compile_website.sh if Google Closure Compiler is installed. Maybe the wiki should start with that if that is the preferred approach? If you feel copying the folder shouldn't be part of flattening, that's fine, please ignore the PR. Moving the code there instead of in the sh file would work for both scenarios though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working minor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants