search

HackSheffield / old-website

The code that powered our website for HS3 and HS4
4 stars 1 forks source link

Implement teams hack from October 2017 #10

Closed boardfish closed 6 years ago

boardfish commented 6 years ago

It's been a long time coming. This is just a straight port of HackSheffield/teams from myself, @mattburman and @p3rzival. Blacklists are unchanged so the following teams within the organisation are displayed:

I suggest we blacklist the Publicity team as you're both already in the current committee and marked as Publicity Officers. It'd also be wise to encourage those who haven't (@IulianLazarina, @SeifELG, @acb17ssp and @AneteZepa) to bring their profiles up to date.

The GITHUB_TOKEN environment variable will need to be set wherever we're deploying the site too, and it adds a couple of dependencies that didn't previously exist.

Closes #1.

robjtede commented 6 years ago

@dannycjones I suggest we add dotenv and initialize from the root app file so the env vars can be placed in a .env file rather than needing to be put in the service file.

robjtede commented 6 years ago

I also agree with blacklisting publicity. The team is only there for repo access control.

dannycjones commented 6 years ago

I'm worried about throwing an error if no GH token is provided. This currently means anyone wanting to make their own update to the website has to register a GitHub Access Token and supply it as an environment variable.

https://github.com/HackSheffield/website/blob/b34ed6583485b1cf499f0204fc9f16e1b72a296d/routes/index.js#L7

Perhaps a warning might be better in the logs - the website will continue to work 'silently' but with a semi-big yelp in the logs?

What do you think @robjtede @TheWispy?

boardfish commented 6 years ago

Alright, I'll see if I can't put a few dents in this today. @robjtede If you're at the LAN this weekend, we could probably take a look at whatever's left afterwards.

boardfish commented 6 years ago

TODO: extract into mixin and create subsections rather than sections. I think the GitHub token error is something that needs working out too before this gets merged.

Just replacing section with subsection hasn't done anything, nor has removing classes from the section element - what's the right way about this?

robjtede commented 6 years ago

@boardfish yeah i'll be around this weekend we can have a good look

robjtede commented 6 years ago

@dannycjones I think it's a good idea but for a different PR.

robjtede commented 6 years ago

@dannycjones Ready to merge; other concerns can be addressed in other PRs imo.

dannycjones commented 6 years ago

It should just need a rebase/merge and be ready to merge in to dev 👍