search

CodeYourFuture / curriculum

The CYF Curriculum
https://curriculum.codeyourfuture.io
Other
34 stars 47 forks source link

Feature: Code Review Web Component #276

Open SallyMcGrath opened 1 year ago

SallyMcGrath commented 1 year ago 
          > This is cool!

A couple of questions:

  1. Is there a nice way to make this filter / group by city? Maybe we could tab by city? Or add a "choose a city" drop-down? As a volunteer attending class in London, ideally I want to be filtering to the London PRs. If we assume people are using the correct PR titles, this feels possible?

If these were labelled, it would be super easy to group by label as a static component or a dynamic one. It would probably be better to write a labeller as a github action than parse and group in Hugo based on title strings. (It's not impossible to do this, but it would be a globby chunk of logic in a template view and ideally we'd limit that much business logic in templates. )

In general, as different cohorts are going through at different times, it will do this implicitly, as you will be looking at the html day plan when your cohort is doing html, and the js one when they're doing that, and it's picking the top 5. I would like a labeller though, because I'd like the action to reject PRs that aren't titled correctly. If you write that action I will revise this as a web component. (And link your example in this Project Briefing)?

  1. Is there a cute way to make this dynamic? Ideally this is a piece of content we'd want to fresh load on page-load, rather than be stale based on when the site happened to publish... I'm not sure there's a nice way to do that, though?
  1. Yes we can make it dynamic by writing it as a web component and doing the fetch inside the component instead of at build. This makes the page heavier but not too bad.

Let's merge this as MVP and write a new ticket with the new features?

Originally posted by @SallyMcGrath in https://github.com/CodeYourFuture/curriculum/issues/267#issuecomment-1717083037

illicitonion commented 1 year ago

Cool cool, sounds good. I will add making an Action to my backlog, not sure when exactly it'll pop to the top of it.

Roughly the logic we want is: When a PR is opened, parse its title. If it is of the expected form, add a label for the cohort number. If it is not, post a comment to the PR saying something's wrong.

And for this to potentially be the basis for others to add more functionality to the action, or to fork it and perform other checks in their own action.

SallyMcGrath commented 1 year ago

Cool cool, sounds good. I will add making an Action to my backlog, not sure when exactly it'll pop to the top of it.

Roughly the logic we want is: When a PR is opened, parse its title. If it is of the expected form, add a label for the cohort number. If it is not, post a comment to the PR saying something's wrong.

And for this to potentially be the basis for others to add more functionality to the action, or to fork it and perform other checks in their own action.

Yes - actually if possible post a comment and set the PR to draft, so it isn't tracked as completed by the trainee tracker?