-
Notifications
You must be signed in to change notification settings - Fork 2
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
Move project to React #5
Conversation
…d connected node options in USB route
…nality (not yet matching to API)
A few things on top of my mind:
|
1. Set up `bmcd-api-mock` for development: | ||
- Clone the [bmcd-api-mock](https://github.com/barrenechea/bmcd-api-mock) repository. | ||
- Follow the instructions in the `bmcd-api-mock` repository to set up and run the mock server. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if I should leave this in. It helped me develop the new app, but it's not a "first-party solution" from Turing. Open to suggestions!
I guess an option would be for the dev to target its own cluster. I'm not sure if CORS restrictions apply there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yea, I did something similar with enabling CORS and, therefore, running the web page on my local machine, passing the API calls to the Turing board.
It could be useful, not sure if you are willing to donate it. But it would at least be worthy of a comment about its existence in the docs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd be happy to donate the code, although I'd like to polish it a bit, like making it more modular and more "to-the-spec", specifically on error handling 😄
@barrenechea That's really impressive! I'll take it for a test spin over the next couple of days. If it looks good, we can merge it as far as I'm concerned. |
that is perfectly fine |
@barrenechea been testing it a bit, and it is an improvement over our last UI. I'm happy to merge it. I however have 2 small comments: |
Thank you for checking it out @svenrademakers ! Regarding fan control, it should only show up if at least one fan is detected (it should never render empty). Do you have the fan chip soldered in? I'd appreciate it if you could give me the answer you're getting for And the node information may be my mock API being out-of-spec on the |
The issue with I'll give it a test and push it to this branch if it fixes it 😄 |
Im using a v2.5.1. which uses a PWM signal to directly to drive the FAN. (this driver is however not fixed yet). From a linux perspective you can assume the thermal driver is not loaded. Perhaps in this case we want to disable the whole fan control ?
curl "https://turingpi.local/api/bmc?opt=get&type=cooling" --header "Authorization: Basic cm9vdDp0dXJpbmc=" -k
{"response":[{"result":[]}]}⏎ |
Got it! It was a quick fix. I was validating for a truthy value to render that area (and it seems an empty array is truthy 😆). It gets fixed if I explicitly check for |
I'm currently building a firmware with the latest changes, the artifact should be finished in an hour or so https://github.com/barrenechea/BMC-Firmware/actions/runs/8940379219 EDIT: I noticed the DTS files for v2.5.1 are not merged yet, my build may not work for the new board 😞 |
Everything ok now! thanks for all your effort and patience :) |
created tag |
I noticed there is no GitHub Release for v3.0.0, with the new pipeline triggered for v2.1.0, but it seems the GitHub release was deleted. The pipeline checks for the I see two options:
Let me know what you think! |
Oh, by the way, whatever is set on package.json's Of course, the app and pipeline can be adjusted to fetch the tag during build time (probably as an env var) and use that instead of package.json to embed the correct version. |
Ah, I forgot that you also updated the GitHub pipeline. I like how you automated it. I'm inclined to change it to an However... |
Everything looks perfect now! I'll update my PR over Thank you very much for your support on this! |
This is an extensive pull request. If approved, feel free to squash it at the end since it comprises many commits 😄 huge thanks to @frankurk for helping me on the CSS side!
The goal is to move the project into React and change how production builds are distributed. I updated the README.md to detail the development process, how production builds work, and contribution restrictions (mainly build size).
jQuery -> React
The goal was to keep the original interface as untouched as possible while adding new features and not going wild on the build size.
disabled
state, or the hostname not being replaced in the header).Removing Basic Auth and the SpA handling routes requires a supporting PR over
bmcd
: turing-machines/bmcd#86The current React bundle size is
1194kb
.The current jQuery project's
dist
folder is1277kb
.Quality Checks
I added a
quality.yml
pipeline that does several basic checks for every upcoming pull request, such as:Distribution process
Pushing the
dist
folder directly to the repo is not recommended, although I understood the usefulness of$(call github,...)
in the Makefiles.Leveraging GitHub Releases with version tracking and automated builds would solve the issue of having fixed static builds and predictable URLs to get them.
I added the
tag-and-build.yml
workflow to leverage the build and release according to the package.json'sversion
field.An example release can be found here.
I'll open a supporting PR overturing-machines/BMC-Firmware#198BMC-Firmware
to download the file from GitHub Releases.Screenshots (running on my cluster 😜)
/login view /info view /nodes view /usb view /firmware-upgrade view /flash-node view /about view Any non-existent client route (excludes /api/bmc routes)Fixes #4