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.

Sign up for GitHub

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

Jump to bottom

Manual QR Code Update #715

Merged
merged 2 commits into from
Jun 12, 2024
Merged

Manual QR Code Update #715

merged 2 commits into from
Jun 12, 2024

Conversation

SteveMicroNova
Copy link
Contributor

@SteveMicroNova SteveMicroNova commented May 10, 2024 •
edited

What does this change intend to accomplish?

Update QR Codes in manual to have branded features, add new QR codes for the upcoming video

Checklist

  • If applicable, have you updated the documentation/manual?

@SteveMicroNova
Copy link
Contributor Author

Requesting review from Jason because he has opinions on the manual branding, requesting review from Lincoln and Ryan because they both have strong opinions about images in git

@SteveMicroNova
Copy link
Contributor Author

Removing requested reviews, going to regenerate the QR codes using a linux library instead of a third party software

@SteveMicroNova
Copy link
Contributor Author

Requesting review from Jason because he has opinions on the manual branding, requesting review from Lincoln and Ryan because they both have strong opinions about images in git

Rerequesting the same three for the same reasons now that the QRs have been updated once more

rtertiaer
rtertiaer requested changes May 15, 2024
View reviewed changes
docs/manual/imgs/apps-qr.png Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

image

you took a smaller image and upscaled it. Please do the inverse - create a large QR code and then create smaller images from it. This ensures the images look great in every resolution. (The attached image is a close up of one of the borders of this image; there should be zero grey in a QR code.)

rtertiaer
rtertiaer reviewed May 15, 2024
View reviewed changes
Copy link
Contributor

@rtertiaer rtertiaer left a comment

Choose a reason for hiding this comment

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

is there a higher quality AmpliPi logo you can use?

Is there a compelling reason why we should include the imaging instructions? even in hackable DIY products I've purchased, that's not something prominently featured in the in-box documentation (and if it were, I would not have the highest expectation of quality.)

rtertiaer
rtertiaer reviewed May 15, 2024
View reviewed changes
Copy link
Contributor

@rtertiaer rtertiaer left a comment

Choose a reason for hiding this comment

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

I like this rendition. You've asked for other feedback too, so I'll leave it to those folks to click "approve"

@linknum23
Copy link
Contributor

How are we creating the pdf? The script/typeset_manual is broken

@SteveMicroNova
Copy link
Contributor Author

How are we creating the pdf? The script/typeset_manual is broken

Is it? I just right click docker-compose.yaml with docker desktop open and hit "Compose up" and I get a PDF

@linknum23
Copy link
Contributor

Looks like a couple of things are wrong with my setup. Give me a little bit to get back up and running.

While I'm having errors it looks like there is still one generated file that needs to be added to .gitignore and removed:

  • docs/manual/manual.fls

@SteveMicroNova
Copy link
Contributor Author

Looks like a couple of things are wrong with my setup. Give me a little bit to get back up and running.

While I'm having errors it looks like there is still one generated file that needs to be added to .gitignore and removed:

  • docs/manual/manual.fls

https://github.com/micro-nova/AmpliPi/blob/ManualQRUpdate/.gitignore
It's actually in there, at the bottom. I've having issues with some files ignoring their presence on the gitignore, am I maybe doing something obviously wrong for them to be continuously uploaded regardless?

@linknum23
Copy link
Contributor

You have to delete the file with ``git rm docs/manual/manual.fls`. After that git will respect the ignore file.

@linknum23
Copy link
Contributor

It looks like the video link is redirected to amplipi's GitHub. I assume that's intended?

@linknum23
Copy link
Contributor

Here's my build of the latest PDF.
manual.pdf

linknum23
linknum23 approved these changes May 15, 2024
View reviewed changes
Copy link
Contributor

@linknum23 linknum23 left a comment

Choose a reason for hiding this comment

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

Looks good to me. I'll have to add a pr to fix scripts/typeset_manual

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 49.65%. Comparing base (b516bb7) to head (d9b9a86).
Report is 51 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #715      +/-   ##
==========================================
- Coverage   50.90%   49.65%   -1.26%     
==========================================
  Files          25       26       +1     
  Lines        5838     6322     +484     
==========================================
+ Hits         2972     3139     +167     
- Misses       2866     3183     +317
Flag Coverage Δ
unittests 49.65% <ø> (-1.26%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

gorski123
gorski123 approved these changes Jun 4, 2024
View reviewed changes
Copy link
Contributor

@gorski123 gorski123 left a comment

Choose a reason for hiding this comment

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

I like the QR codes, The quick start video QR code appears to be broken but I'm guessing this is more a Ryan / Andrew question.

@SteveMicroNova
Copy link
Contributor Author

I'm guessing this is more a Ryan / Andrew question.

It's a Steve question, I made the cloud flare reroute, generated the QR codes, and added the image they've got on them so I've got it on lock
You'll have to define "doesn't work" for me however; does it not scan, or does it not go where expected?
I'll hop on this tomorrow

@rtertiaer
Copy link
Contributor

@gorski123 do we have a quickstart video to point to? right now it redirects me to the manual but that's intentional.

@SteveMicroNova
Update QR Codes in manual to have branded features
1831748 
Update QR Codes

Remove unused QR codes, replace a few with SVGs instead of PNGs

Better quality logo
REmove reimaging instructions
Update manual
Remove files that should be gitignored

remove generated file

Update QRs again
@SteveMicroNova
Update QR code with still-active domain
46f26c6
@SteveMicroNova SteveMicroNova merged commit c8a3847 into main Jun 12, 2024
3 checks passed
@SteveMicroNova SteveMicroNova deleted the ManualQRUpdate branch June 12, 2024 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@linknum23 linknum23 linknum23 approved these changes

@rtertiaer rtertiaer rtertiaer approved these changes

@gorski123 gorski123 gorski123 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@SteveMicroNova @linknum23 @codecov-commenter @rtertiaer @gorski123