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

Project: Restaurant Page: Added section on .gitignore templates #27973

Merged
merged 2 commits into from
May 17, 2024

Conversation

Etharialle
Copy link
Contributor

Because

The Restuarant Page lesson introduces the .gitignore file, but does not mention anything about .gitignore templates which are part of the GitHub web interface when creating a repo. Specifically for JavaScript the common template is actually named node which may not be intuitive to students.

This PR

  • Added section .gitignore templates and brief description.

Issue

N/A

Additional Information

Pull Request Requirements

  • I have thoroughly read and understand The Odin Project curriculum contributing guide
  • The title of this PR follows the location of change: brief description of change format, e.g. Intro to HTML and CSS lesson: Fix link text
  • The Because section summarizes the reason for this PR
  • The This PR section has a bullet point list describing the changes in this PR
  • If this PR addresses an open issue, it is linked in the Issue section
  • If any lesson files are included in this PR, they have been previewed with the Markdown preview tool to ensure it is formatted correctly
  • If any lesson files are included in this PR, they follow the Layout Style Guide

@github-actions github-actions bot added the Content: JavaScript Involves the JavaScript course label May 15, 2024
@CouchofTomato
Copy link
Member

There is a linting issue that needs to be fixed before this can be merged. A heading needs a blank line above and below. Please amend.

@MaoShizhong
Copy link
Contributor

Do you think we need to include this in the project content?
All we need is to have node_modules and dist in a .gitignore, which requires introducing what .gitignore is anyway, and the node template includes a lot more than that. Won't actually cause any problems since it won't ignore stuff we don't want ignored, and it includes both node_modules and dist. I'm just wondering if it's actually significantly valuable to include beyond what's already introduced.

Side note, there's a typo .gitnore

@CouchofTomato
Copy link
Member

@MaoShizhong

Yeah I did consider your exact thoughts although thanks for picking up the typo. The reason I felt it was a good addition is I think it's useful for students to know that templates exist when they start their own projects in future. We could maybe add that we don't think students should use one early on but just knowing you can do it seems pretty useful to me.

@MaoShizhong
Copy link
Contributor

MaoShizhong commented May 15, 2024

That's fair enough @CouchofTomato @Etharialle
What do you both think about possible just adding the note on those templates to the end of the preceding .gitignore section? As opposed to making a whole new subsection itself. To me it feels better as just an extension of the previous part, like a "When you create a new repo, there's an option to include a gitignore template, where the node template includes node_modules and dist already."

Also because I just noticed in that previous part, on top of node_modules being ignored for getting big, dist is also commonly ignored because it's not needed (can be generated by any user by bundling) but we don't mention dist. Just found it odd we only mention node_modules there then in the project steps we say both node_modules and dist.

@Etharialle
Copy link
Contributor Author

I'll make the linting changes and typo correction shortly. I didn't want to go too far down the rabbit hole on gitignore templates, but since it's something students see every time they create a repo I thought it would be worth mentioning (especially since the JavaScript related template is called node instead of JavaScript).

That said it's not crucial that it is here and if there is a better place or if it is felt that it is unnecessary this PR can be closed.

@MaoShizhong
Copy link
Contributor

I'll make the linting changes and typo correction shortly. I didn't want to go too far down the rabbit hole on gitignore templates, but since it's something students see every time they create a repo I thought it would be worth mentioning (especially since the JavaScript related template is called node instead of JavaScript).

That said it's not crucial that it is here and if there is a better place or if it is felt that it is unnecessary this PR can be closed.

Nah, no need to close the PR. Just offering an opinion above for discussion, but it's not a blocking opinion

@CouchofTomato
Copy link
Member

Hey @Etharialle

Do you think it is worth making the point that for smaller projects it's unlikely to be useful to use a template vs just adding the directories/files to your own gitignore? We don't want people to go crazy using a template when they'd only gitignore node_modules

@Etharialle
Copy link
Contributor Author

I'm not sure. I typically use a template when creating a project so I don't have to remember to manually create a .gitignore file. I don't think there is much concern about having extra lines in the .gitignore.

My main concern would be with projects that include a .env file with secrets in it that are accidentally tracked because a a .gitignore wasn't created or it was manually created, but .env wasn't added.

@CouchofTomato
Copy link
Member

Fair enough. Thanks for your contribution.

@CouchofTomato CouchofTomato merged commit 8b5710a into TheOdinProject:main May 17, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content: JavaScript Involves the JavaScript course
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants