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.

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

Pacman tutorial website #51

Merged
merged 48 commits into from
Nov 26, 2024
Merged

Pacman tutorial website #51

merged 48 commits into from
Nov 26, 2024

Conversation

orzechow
Copy link
Member

@orzechow orzechow commented Oct 4, 2024

Let's split a first GitHub pages website from the pacman tutorial website.
This adds the Tutorial

Based on

Closes #28

Depends on #53

@orzechow orzechow changed the base branch from main to add_website October 4, 2024 14:17
@orzechow orzechow mentioned this pull request Oct 4, 2024
Base automatically changed from add_website to main October 11, 2024 14:38
@ll-nick ll-nick changed the base branch from main to pacman-demo October 18, 2024 18:30
@ll-nick
Copy link
Collaborator

ll-nick commented Oct 18, 2024

I just changed the base to the demo branch and hopefully maybe didn't break stuff during the rebase.

@ll-nick
Copy link
Collaborator

ll-nick commented Oct 18, 2024

Come to think of it, that didn't actually make too much sense. I changed to the demo branch because we need to remove some code from the demo so that it can be re-implemented.

But of course that part should be merged to main. I'll try and undo the rebase I just did.

@ll-nick ll-nick changed the base branch from pacman-demo to main October 18, 2024 18:45
@orzechow
Copy link
Member Author

@ll-nick I fixed the git history by

  • rebasing my state 1132327
  • onto origin/main ae7038e and
  • cherry-picking your two commits on top it.

@ll-nick ll-nick force-pushed the pacman-tutorial-website branch 3 times, most recently from 1c1d1ea to 7dfac0e Compare October 24, 2024 14:22
@ll-nick
Copy link
Collaborator

ll-nick commented Oct 24, 2024

Alrighty, here's a first draft, let me know what you think :)

The tutorial branch is all setup, too.

@ll-nick ll-nick marked this pull request as ready for review October 24, 2024 14:25
@ll-nick ll-nick self-assigned this Oct 24, 2024
@ll-nick
Copy link
Collaborator

ll-nick commented Oct 24, 2024

@orzechow I can't actually add you as a reviewer since you opened the PR. I assume you can give feedback anyway and should you be happy, I'll approve my own changes ;-)

@ll-nick ll-nick mentioned this pull request Oct 31, 2024
1 task
@orzechow
Copy link
Member Author

The tutorial quickly starts into the coding part – which I prefer against bloated descriptions 👌

Still, I'm thinking, if we should add a basics section explaining arbitration graphs in general – maybe as a sidelink or so.
IMHO we can't assume visitors are aware of our publication and know the concept.

If short enough, we could add this to the main Readme 🤔

@ll-nick what do you think?

@ll-nick
Copy link
Collaborator

ll-nick commented Nov 14, 2024

Yes, that is a very good point. I think a short and concise description in the main README would be the very least of what we should do.
In the tutorial introduction, we could also link to a "theory" section where we go more in depth about the concepts, although we could also just link a paper there.

@orzechow
Copy link
Member Author

In the tutorial introduction, [...] we could also just link a paper there.

Or paste some paper content in here.

orzechow and others added 26 commits November 26, 2024 20:52
Spell Pac-Man properly

Co-authored-by: ll-nick <[email protected]>
@orzechow
Copy link
Member Author

Ok, I finally got around to reviewing this :) First of all thanks for all the work you put into it, I think the tutorial is pretty awesome!

Thanks a lot for the thorough review and going through the tutorial once more 🙌

  • I agree that it is better without task 3. Just the task numbering needs to be fixed now.

Done: d09deff

  • The Browser title aren't super nice. Right now it says e.g. Arbitration Graphs Tutorial | arbitration_graphs instead of e.g. Arbitration Graphs Tutorial | Task 1

Alright, first draft: cbaf5aa
I'm not super happy with that, because the tagline adds even more redundancy. We could also use the existing menu_title (though shorter and without "Task 1:" prefix)
What do you think?

I won't wait with merging, though. We can further tune that in a small independent PR.

  • The random switching doesn't so well because the random arbitrator switches too often. But I guess that's a separate issue.

Agree 👍

Rebased onto main

@orzechow orzechow merged commit 47793fa into main Nov 26, 2024
1 check passed
@orzechow orzechow deleted the pacman-tutorial-website branch November 26, 2024 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Write tutorial
2 participants