-
Notifications
You must be signed in to change notification settings - Fork 7
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
Batch 1 #94
Batch 1 #94
Conversation
Awesome, thanks! I will review later today |
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.
@Alumniminium thanks for all the work on this, you went through a whole lot and it looks great.
I made several suggestions - please feel free to ignore any that you think are overkill
my apologies, for some reason I didn't review all the files - I must have missed the pager or something, I am looking through the rest now |
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.
ok, reviewed the fiels I had missed
This review makes me want to die |
Sorry, not sure what to tell you - I was just being as thorough as possible, and as I said, some are just suggestions/questions. I am not pushing you to include all of them - perhaps we should call this a collaboration rather than review. Again, thanks for taking on so many of these. If you just don't have time to handle whatever ones are actually more important, just reassign those issues to me, I will merge this, and then pull from edit to make some of the suggested changes. Just let me know and reassign the issues. |
Ready to merge, two new issues created to address things I dont know how to. |
Thanks, will merge and do the other issues today |
* #84 * #84 - fix more grammar issues * #84 - fix more grammar issues * Some more :D * Update #14 * Make changes @root-hal9000 suggested * Updated Document article Among other edits, renamed the "document" concept to be called what it actually is, the database - which could help avoid confusion with the concept of "note" * Changed all references to document to database - Changed references to document to say database - Changed references to "demo document" to "demo notes" - renamed document.md to database.md and replaced in links - Changed location of database in TOC to go under advanced usage Issue #32 * fixed links change during merge conflict * #84 * #83 * #82 * #81 * #81 * #79 * #78 * #75 * #74 * #72 * #71 * #68 * #64 * #61 * Review PR #94 * Review PR #94 - Url Encoding * Add missing images * add missing image to template.md * template-create-instance-note.png * closes #12 * Closes #16 * Closes #17 * Closes #18 * Closes #20 * Closes #31 * clarify the location of options * add tics around `true` and add newline at end of file * Remove period from non-sentence lines * add "(or `viewType` attribute)" * reference docker installation docs directly instead of other third party website. * simplify wording * Update docker image references * Reference new documentation * One more docker reference updated * Remove documenation warning * Remove warning label --------- Co-authored-by: root-hal9000 <2352828+root-hal9000@users.noreply.github.com> Co-authored-by: MeIchthys <github.com@meichthys.com>
Closing: #84
Closing: #83
Closing: #82
Closing: #81
Closing: #80
Closing: #79
Closing: #78
Closing: #75
Closing: #74
Closing: #72
Closing: #71
Closing: #68
Closing: #64
I've restructured most files and rewrote/fixed URLs - either broken internal ones or links to Zadam.
I added %%{WARNING}%% where appropriate (chromestore link to the clipper)
After that, I let it run through GPT-4o-mini to fix up spelling/grammar mistakes I might have introduced myself. I made sure to double-check the output in case it changed steps or made things up.