-
Notifications
You must be signed in to change notification settings - Fork 12
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
stack/intro: Lab 8 - Stiva #22
base: main
Are you sure you want to change the base?
Conversation
Am sa dau fix la erori cam vineri-ish, sau pe weekend |
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.
The content is mostly fine. Most of my inline suggestions apply to all code files. In addition
- Fix the github action test failures [1]. For the spellcheck false positives, you'll have to add the missing words to these wordlists here [2] as a separate PR.
- Squash your commits to a single one named something like
stack/stack: Add lab exercises, guides and reading
+ a description and signature.
[1] https://github.com/cs-pub-ro/hardware-software-interface/pull/22/checks
[2] https://github.com/cs-pub-ro/hardware-software-interface/actions/runs/8829121620/job/24239469518?pr=22
chapters/stack/intro/guides/stack-addressing/support/stack-addressing.asm
Outdated
Show resolved
Hide resolved
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.
Remake this image as .svg
and rename it to process-address-space.svg
. We only use snake_case
for source code file names.
6b2fb61
to
236c61e
Compare
236c61e
to
d4be640
Compare
d4be640
to
e8e692a
Compare
Don't know how to fix spellcheck. Otherwise done 👍 |
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 made inline comments with what needs changing. To fix spellcheck, add the false positives to these wordlists [1] and keep them sorted and avoid duplicates using sort -u -o <file-name> <file-name>
.
[1] https://github.com/open-education-hub/actions/tree/main/spellcheck/config
chapters/stack/intro/drills/tasks/stack-addressing/solution/stack-addressing.asm
Outdated
Show resolved
Hide resolved
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.
Split this file into the following files and group the exercises and demos around them (where possible) so they form subchapters:
introduction.md
(intro + stack data structure)stack-operations.md
stack-addressing.md
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.
The content is quite short, I think splitting up the concepts will just end up with more confused students. Teach them what they need to know, then let them practice. Splitting it off so atomically is just not worth it imho
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 then remove the "Introduction" subchapter in config.yaml
and make the links in the tasks refer to specific sections in the reading material (using #
, such as .../README.md#stack-operations
).
Cool description here Signed-off-by: George Cosma <[email protected]>
e8e692a
to
f28b384
Compare
Prerequisite Checklist
Description of changes
Initial draft done, can be reviewed