fix: clean HTML structure and resolve Prettier CI failures#86
fix: clean HTML structure and resolve Prettier CI failures#86shishir-21 wants to merge 6 commits intogbowne1:masterfrom
Conversation
|
Hi @gbowne1 👋 |
|
I'm not totally sure what to do about the CI. It has several issues but... we need to address the .env-example filename. Theres now several open PRs trying to fix it so its kind of an emergency to fix this issue. Its causing issues in Windows and git. I'm hoping fixing this filename will fix some of the issues. I think I would update the CI configuration. |
|
Got it 👍 |
|
.env PR has been merged. CI fix is likely in #87 by @glenjaysondmello so I would head there first then make appropriate changes to your PR(s) once that's merged @shishir-21 and @Ved178 We're tripping over ourselves due to some issues which should be in the DEVSETUP.MD etc document |
gbowne1
left a comment
There was a problem hiding this comment.
I checked out this change locally for testing and review for merge
It's of my opinion that the correct CI fix is in #87 by @glenjaysondmello
This can wait a little bit to further enhance that PR once it rebased at this point
I'll request changes now pending merge of #87
|
@shishir-21 @Ved178 @glenjaysondmello @abhisheksingh1204 etc I think this could use one more rebase? I merged #92 a pretty big PR which fixed the remaining issues and tests passed. Let's try and get in these issue fixes and PRs soon. I'll re review. |
|
Hello! I’m here because my merged PR was mentioned in this one. Given some of the recurring issues I’ve been seeing—CI failures, package-lock diffs, and linting/formatting inconsistencies—I wanted to propose some potential improvements: 1. Dockerize the codebase 2. Makefile as a command interface
This would give contributors a single, documented entry point and could also simplify the CI script. 3. Frontend / backend separation I’d like to hear what the maintainers think about these ideas and whether they’d be open to this kind of change. If there’s general agreement, I’d be happy to help 👍 |
|
I think a preflight or precommit hook or something that runs before committing especially for lint and format would be better as long as people would. We could use husky? Vite is pretty good about it's own warnings etc during development. We're also gonna have to improve docs too. We also should probably also add playwright or cypress. Cypress is a bit complex. Anyway all discussion is in the Discussion tab of this project I'm not opposed to a docker or even a dev container (VS/VSC). I just don't want to complicate As for moving to a separated environment we are headed there just looking to get all open PRs merged before any real new ones appear, outside of solving the problems we're having. |
gbowne1
left a comment
There was a problem hiding this comment.
I checked out this change locally for testing and review for merge
It looks clean as a eslint / prettier run.
Some minor things but I think the PR is good
Approving this PR for merge pending further review by the other collaborators and maintainers.
Fixed typo in const of service worker
shishir-21
left a comment
There was a problem hiding this comment.
Thanks for the approval and fixes 👍
I see CI is still failing on the build-and-test jobs.
I’ll pull the latest master, rebase again, and run the full CI workflow locally to identify what’s breaking.
I’ll push an update shortly once I isolate the issue.
|
There's still a few files that Prettier went nuts about doctype being lowercase. I don't think it's a big problem either way just should be consistent and well 🤷♂️why it got changed I'm old school. It should be always be DOCTYPE html. A search or grep for lower case doctype should show them. |
|
CI.yml needs a bit of work. I agree with only one node workflow. This particular run was bad package lockfile |
|
Actually.. I think.. we can just close this without merging. I created #99 and #111 and a separate PR for index.html being completely messed up. #113 Will still need minor tweaks afterwards but this should render. I have no idea what messed this up 🤷♂️ Let's review those and get them merged soon. Yours is a much older PR and we have had a bunch of merges since that messed up stuff significantly |
|
Closing this PR as cleanup work is in progress #113 etc |
What this PR does
This PR fixes CI failures caused by invalid HTML structure and formatting issues.
Changes made
npx prettier --check .passes locallyContext
masterResult
✅ Prettier CI checks pass
✅ No HTML syntax errors
✅ Clean, reproducible build