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

Javascript Course: feat: added referenced lessons url for easy navigation #29321

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

devnchill
Copy link
Contributor

Because

Lesson mentions reference to previous lessons. Looking up and going through past lesson isn't of much convenience .
Adding url for the references lesson gives more clarity and provides convenience.

This PR

  • wraps the references with past lesson links for easy navigation.

Issue

Additional Information

I tried linting changed file however i get Linting: 0 files . I don't remember if this is how it was always
Img

Pull Request Requirements

  • [X ] I have thoroughly read and understand The Odin Project curriculum contributing guide
  • [ X] 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
  • [ X] The Because section summarizes the reason for this PR
  • [ X] 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 Jan 15, 2025
@MaoShizhong
Copy link
Contributor

MaoShizhong commented Jan 15, 2025

As per the linting instructions, the command must be run in the repo root else npm won't find the correct files.
Turns out it doesn't have to be run in the repo root, just that the lesson/project file path must be given relative to the repo root (not relative to your CWD). The linting guide has been addressed with this now.

@devnchill
Copy link
Contributor Author

image
it worked thanks

@@ -125,7 +125,7 @@ Doing this can look messy, but it is a very easy way to handle errors without ap

### Practice

Remember the Giphy API practice project? (If not, you should go back and complete the API lesson). We are going to convert the promise based code into `async/await` compatible code. Here's a refresher of the code we are starting with:
Remember the Giphy API [practice project](https://www.theodinproject.com/lessons/node-path-javascript-working-with-apis#lets-do-this)? (If not, you should go back and complete the [API lesson](https://www.theodinproject.com/lessons/node-path-javascript-working-with-apis)). We are going to convert the promise based code into `async/await` compatible code. Here's a refresher of the code we are starting with:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Remember the Giphy API [practice project](https://www.theodinproject.com/lessons/node-path-javascript-working-with-apis#lets-do-this)? (If not, you should go back and complete the [API lesson](https://www.theodinproject.com/lessons/node-path-javascript-working-with-apis)). We are going to convert the promise based code into `async/await` compatible code. Here's a refresher of the code we are starting with:
Remember the [Giphy API practice project](https://www.theodinproject.com/lessons/node-path-javascript-working-with-apis#lets-do-this) from the previous lesson? We are going to convert the promise based code into `async/await` compatible code. Here's a refresher of the code we are starting with:

Just a suggestion to further enhance the change:

  • "practice project" isn't accessible enough of a link text label but "Giphy API practice project" would be.
  • We don't really need to link to both the Giphy section and the API lesson right next to each other, a bit redundant. The Giphy section is more relevant and the API lesson could just be referred to as the "previous lesson" which IMO flows a bit better anyway.
  • Shouldn't need to say "if not, you should go back" - the curriculum expects to be done in order anyway so people shouldn't be skipping to this lesson without having done the API one. In other lessons where there was this sort of language, those parts were removed because it just didn't make sense to accommodate for potential skipping.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I won't be able to contribute further at the moment. I’m currently occupied with exams for the next couple of months, so now that this has been reviewed, anyone else is welcome to take over the pr...

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.

2 participants