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

Sub step about closing TOP in Windows and reminder to follow Linux instructions. #28356

Merged
merged 4 commits into from
Jul 12, 2024

Conversation

LAKSHYA201
Copy link
Contributor

Because

As mentioned in the issue , regardless of many sentences throughout the beginning that asked the users to do everything in VM have not been effective , so a specific highlighted sub step was required . There was also need of a reminder to be added that learners only follow the Linux instructions from now on.

This PR

  • A sub step 4.1 stating asking the user to close TOP in windows and open it in the VM along with a summarized explanations has been added .
  • A "Remember" has been added to ask learners to follow only the linux instructions.

Issue

Closes #28325

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: Foundations Involves the Foundations content label Jul 7, 2024
@KevinMulhern KevinMulhern requested review from a team and xandora and removed request for a team July 8, 2024 12:03
@LAKSHYA201
Copy link
Contributor Author

LAKSHYA201 commented Jul 8, 2024

upon running npm run lint :project -- "path" , there is no error but running lint:lesson produces an error : Required heading structure [Expected: ### Lesson overview; Actual: #### Be mindful of the OS you are using].

is there something i should do about this or can do about this ?
should i perhaps remove the #### and put a Remember::-Be mindful of the OS you are using , if it would help ?

@MaoShizhong
Copy link
Contributor

upon running npm run lint :project -- "path" , there is no error but running lint:lesson produces an error : Required heading structure [Expected: ### Lesson overview; Actual: #### Be mindful of the OS you are using].

is there something i should do about this or can do about this ? should i perhaps remove the #### and put a Remember::-Be mindful of the OS you are using , if it would help ?

For this lint error, could you replace the first line with the following:

<!-- TODO: Revisit lesson/heading structure to remove need to disable rules -->
<!-- markdownlint-disable MD024 TOP004 -->

The error's flagging because the current disable comment targets the wrong rule code, but we also need the TODO because this (and a few other installation lessons) have structures we need to rethink for accessibility reasons. They're lower priority for now, but we need the TODOs for when we get to them.

Copy link
Member

@xandora xandora left a comment

Choose a reason for hiding this comment

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

Overall, looks good to me! Just wondering about the escaped underscore...

foundations/installations/installations.md Outdated Show resolved Hide resolved
foundations/installations/installations.md Show resolved Hide resolved
Copy link
Member

@xandora xandora left a comment

Choose a reason for hiding this comment

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

I don't have any further issues with this.

@LAKSHYA201
Copy link
Contributor Author

Hi @MaoShizhong , sorry for abruption but i have solved the issue and wanted to remind you that the PR is ready to be merged, just in case it slipped the maintainers' radars.

@MaoShizhong
Copy link
Contributor

@xandora Forget to hit merge for this? 😜

@MaoShizhong MaoShizhong merged commit 3ca36d2 into TheOdinProject:main Jul 12, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content: Foundations Involves the Foundations content
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Installations: Clarify that copy pasting should happen in the VM
3 participants