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

Update deployment.md #27093

Merged
merged 3 commits into from
Jan 19, 2024
Merged

Conversation

kamranzafar4343
Copy link
Contributor

@kamranzafar4343 kamranzafar4343 commented Jan 9, 2024

Because

Codeblocks should be updated to align with new changes in the layout style guide

This PR

Remove aforementioned link from the Node path's deployment lesson

Issue

Related to #26939

Pull Request Requirements

  • I have thoroughly read and understand The Odin Project 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

removed the aforementioned link
@github-actions github-actions bot added the Content: NodeJS Involves the NodeJS course label Jan 9, 2024
@01zulfi 01zulfi requested review from a team and fortypercenttitanium and removed request for a team January 9, 2024 13:35
Copy link
Contributor Author

@kamranzafar4343 kamranzafar4343 left a comment

Choose a reason for hiding this comment

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

It looks fine. passed

Copy link
Contributor

@fortypercenttitanium fortypercenttitanium left a comment

Choose a reason for hiding this comment

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

You removed the link for the video.

kamranzafar4343

This comment was marked as off-topic.

@fortypercenttitanium
Copy link
Contributor

@kamranzafar4343 are you going to put the link back in? If not, I'm going to have to close this PR.

@kamranzafar4343
Copy link
Contributor Author

Why to put link back in?

@fortypercenttitanium
Copy link
Contributor

fortypercenttitanium commented Jan 18, 2024

Why to put link back in?

You removed a link in the lesson for "GoNodeJS video guide for deploying NodeJS applications to Fly.io". Why did you do that?

@kamranzafar4343
Copy link
Contributor Author

kamranzafar4343 commented Jan 18, 2024

"Remove aforementioned link from the Node path's deployment lesson"
I understood that it says to remove the link from Node path's deployment lesson, can you tell me what this PR says? & I will put in back if this PR saying something else.

@fortypercenttitanium
Copy link
Contributor

"Remove aforementioned link from the Node path's deployment lesson" I understood that it says to remove the link from Node path's deployment lesson, can you tell me what this PR says? & I will put in back if this PR saying something else.

You wrote that in the PR description. Why is it being removed? The issue you linked has to do with markdown styling. I'm very confused why this link is being removed from the lesson. Is there a different issue I should be looking at?

@kamranzafar4343

This comment was marked as off-topic.

@kamranzafar4343
Copy link
Contributor Author

"Remove aforementioned link from the Node path's deployment lesson" I understood that it says to remove the link from Node path's deployment lesson, can you tell me what this PR says? & I will put in back if this PR saying something else.

You wrote that in the PR description. Why is it being removed? The issue you linked has to do with markdown styling. I'm very confused why this link is being removed from the lesson. Is there a different issue I should be looking at?

I think the probelm is caused by linking a different issue with PR.

@kamranzafar4343
Copy link
Contributor Author

@fortypercenttitanium the issue was solved by linking the relevant issue with this PR.

@fortypercenttitanium
Copy link
Contributor

@fortypercenttitanium the issue was solved by linking the relevant issue with this PR.

Okay I see now. That wasn't the issue that was linked originally!

You need to remove the words inside the square brackets on line 112 as well.

@fortypercenttitanium fortypercenttitanium merged commit f48f2a1 into TheOdinProject:main Jan 19, 2024
@kamranzafar4343
Copy link
Contributor Author

Is this okay that some checks have not passed?
Can i make an issue on this?

@kamranzafar4343
Copy link
Contributor Author

@fortypercenttitanium
Copy link
Contributor

It's fine - it's a new action we implemented and fixing those is out of scope for your issue.

@kamranzafar4343
Copy link
Contributor Author

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content: NodeJS Involves the NodeJS course
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants