Skip to content

added contributing page #97

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

Merged
merged 6 commits into from
May 12, 2023
Merged

added contributing page #97

merged 6 commits into from
May 12, 2023

Conversation

matthewcn56
Copy link
Member

Made the contributing page and included a couple things:

  • definition of opensource
  • list of uclaacm's open source projects with good-first-issues from octokit
  • quickstart contribution git/github guide

Also adjusted the project request functions and turned smaller tasks into functions to reduce code smells.

@matthewcn56 matthewcn56 requested a review from mattxwang March 26, 2022 05:47
Copy link
Member

@mattxwang mattxwang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Matt, great work on getting this page up and running! I have some broad areas of feedback:

  • copy (which I think is really important) - mostly threaded, though I didn't really give feedback on the contribution workflow section. I think conciseness and simple language is super important for this page.
  • the "good first issue" feature - below

Good First Issue

I really like the Good First Issue idea! Some suggestions:

I have some minor styling feedback, but that's probably not necessary for this PR. Great work as always!

how can i contribute?
</h2>
<p>
We&apos;ll quickly go over how we can use git/github to work on some open source projects, and if
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where are you going over how to use git/GitHub?

(also, you can probably split up the run-on sentence here)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We quickly go over cloning, forking, branching, etc. But, it could probably be hashed out better (maybe in a content PR?)

Comment on lines 83 to 90
<h2 id='good-first-issues'>
acm&apos;s projects w/ good first issues
</h2>
<div className='card'>
<div className='card-body'>
{displayedgfiProjects}
</div>
</div>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a pretty large content box; I would suggest either making it collapsible, only showing the first 3, or linking to some other page. If I saw this, I wouldn't know to scroll down and read the other FAQs!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think putting it at the end of the page and linking to it or making it its separate page would work, would love to hear your thoughts!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a huge fan of end of page now looking at it. I think separate page makes the most sense, and add it to the navbar/the front four cards (since some of those links are meaningless right now).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, we'll definitely have to figure out how to sort out the navbar if we'll have the ucla-opensource page up there as well, it might get a bit crowded. I'll probably add a link to it on the front-page for now.

</div>
</div>

<h2>contribution workflow</h2>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this section is good, but you can move it to a new page if you'd like; I would say it's different in style from the other things on this page (and it's getting long). Perhaps "making your first pull request".

Also, most of your examples use the CLI. You may want to explain:

  • how to download git if you don't have it already (ex, most Windows systems)
  • what a CLI is (particularly important for Windows users who may want WSL or will have to learn powershell)
  • if they should use something like GitHub Desktop (this is opinionated)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I think the fixing up of this and deciding how we want to structure it can be its own issue and PR as well, but up to you!

restructured the gfi projects, and addressed some content changes
@matthewcn56 matthewcn56 requested a review from mattxwang July 9, 2022 08:47
Copy link
Member

@mattxwang mattxwang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not going to comment too much on the copy / markup if you want to improve it later, but I would definitely suggest giving it a serious pass (being consistent with capitalization and using code blocks, being more concise, breaking things up into reasonable chunks, etc.). This is more of a design and communications task and less of a developer one, but probably more important than the actual functionality of this page. I think you should really invest some resources into this if you want to get this ready for fall.

I think I still agree with my previous feedback on not writing to a file as a part of this project; I don't think it's the appropriate use-case of Next. I wonder if you should investigate deploying on vercel, using an in-memory cache, or fixing whatever issue you had with SSR and revalidate timers (are you actually deploying with SSR? make sure!).

</h1>
<div className='row spaced-row'>
<h1>get started </h1>
<a href='#good-first-issues'><button>good first issues</button></a>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like wrong markup?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean with the space in the h1 or the button being in the same row as the h1? At first I had the button to the GFI projects on the same line as the header but I'll change that to point to a new page now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah no, in this case I mean having a button inside the anchor. I know it's tied to CSS but it's an accessibillity no-no.

(I know it's a little hypocritical since I banged out the first iteration of this code)

@matthewcn56
Copy link
Member Author

using an in-memory cache

Interesting, I'll see if we can do this stuff with Netlify but the main problem that I see happening is that currently as it is, one person can get the data but if multiple people go to the website then we might overload the system as is :((

I'll look to see if isr is actually being used haha, since it seems as though it's not working as intended. I might ask for some help in the future but i'll lyk!

@matthewcn56
Copy link
Member Author

Not going to comment too much on the copy / markup if you want to improve it later, but I would definitely suggest giving it a serious pass (being consistent with capitalization and using code blocks, being more concise, breaking things up into reasonable chunks, etc.). This is more of a design and communications task and less of a developer one, but probably more important than the actual functionality of this page. I think you should really invest some resources into this if you want to get this ready for fall.

I'll def get some opensource enthusiasts to flesh this out and represent opensource in the best light! I'll loop Jason Cheng @jc65536 into this to see if he wants to talk about opensource on this page!

@mattxwang
Copy link
Member

if multiple people go to the website then we might overload the system as is :((

I really think we're using Netlify wrong; our use-case is so small compared to many other companies, projects, etc. that use it - this is such a common business-case problem (this is one of the driving examples of SSR/ISR/etc.) so I think there's some more research we should do. Good luck on figuring it out, I think this can really pay dividends for the rest of ACM though!

@matthewcn56
Copy link
Member Author

(are you actually deploying with SSR? make sure!).

Oop I just checked our netlify page for opensource and it looks like we're currently on v3.9.2 for the netlify nextjs plugin but ISR support only started on v4.0 and up! Updating it rn

@matthewcn56 matthewcn56 requested a review from AdvitDeepak May 12, 2023 01:35
Copy link

@AdvitDeepak AdvitDeepak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job, looks perfect!

@AdvitDeepak AdvitDeepak merged commit fce7103 into main May 12, 2023
@AdvitDeepak AdvitDeepak deleted the contributing branch May 12, 2023 01:38
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.

3 participants