-
Notifications
You must be signed in to change notification settings - Fork 1
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
Update AMDS Firmware page for new updates made in AMDC v1.3 #99
Conversation
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.
Thanks @codecubepi ! Looking nice! a few comments:
Feedback has been addressed (except for the one above), and I've confirmed that everything looks good by building locally. |
Alright, I added the additional descriptions you requested. Again, I tested to make sure my fancy color-coded bullets build correctly. Even if everything looks good now (ie you approve), we should probably hold off on merging this until v1.3 is fully de-bugged and released. |
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.
Nice, I approve.
I agree, let's wait to hit the merge button until v1.3
is released, i.e., Severson-Group/AMDC-Firmware#372
Switching back to Draft status so that I remember to update the AMDS docs with the new debug counters: |
@npetersen2 This is now also ready for review, as I've added the new text and images pertaining to the debug counters to the AMDS User Guide page. This should be merged after #108. |
@npetersen2 this PR is still waiting for a review. It looks like we have a merge conflict on the User Guide page, since Ryan merged his math profiling docs the other day. It is an easy conflict that I'll fix via the web editor when it's time to merge. |
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.
Looks good @codecubepi
My only comment would be to regenerate the plot with increased font size to ensure it will be legible on the website. But, it is probably fine... :)
I built the page and think the plot text is pretty legible... then again I am always teased for liking my text small 🤷♂️ |
Resolves #98
I updated any pages about the AMDS that had outdated information, whether it be changes in AMDC Firmware v1.3, or older.