Skip to content

Refactor AWS Deployment Guide + Block Code Format #879

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

Closed
wants to merge 13 commits into from
Closed

Refactor AWS Deployment Guide + Block Code Format #879

wants to merge 13 commits into from

Conversation

MrMartinR
Copy link
Contributor

No description provided.

@MrMartinR
Copy link
Contributor Author

actually I saw this PR #840 and I probably have to change the json to json5 🤔...

@dblythy
Copy link
Member

dblythy commented Jun 25, 2022

Thanks for this PR @MrMartinR! Does specifying json make much of a difference compared to specifying js?

@MrMartinR
Copy link
Contributor Author

Hi @dblythy as far as I know, allows comments in block code, (actually I have another PR with that changes)

@dblythy
Copy link
Member

dblythy commented Jun 25, 2022

Could you possibly send a screenshot of the docs before vs after with the changes? I don’t quite understand the value of the changes

@MrMartinR
Copy link
Contributor Author

Screen Shot 2022-06-25 at 19 10 25

Screen Shot 2022-06-25 at 19 10 12

@dblythy
Copy link
Member

dblythy commented Jun 25, 2022

I understand the difference in code; I’m referring to the difference in the way the outputted docs look (you can replicate this with npm run docs and comparing to parseplatform.org). Perhaps I don’t quite understand what the issue that this PR fixes, @MrMartinR feel free to enlighten me 😊

@MrMartinR
Copy link
Contributor Author

behind the vscode is the output, actually, json5 allow comments, but removes the color label/value another example but in MD file in GitHub is this...
Screen Shot 2022-06-25 at 19 13 11

@MrMartinR
Copy link
Contributor Author

Screen Shot 2022-06-25 at 19 20 25

@MrMartinR
Copy link
Contributor Author

Ok, find a solution... using ```jsonc (json with comments) keeps the highligts and not complain about the comments...
image

@dblythy
Copy link
Member

dblythy commented Jun 30, 2022

Sorry @MrMartinR I was struggling to see. Looks good though!! So should we use jsonc instead?

@MrMartinR
Copy link
Contributor Author

Yep, looks like jsonc is the best solution, in this PR I changed from js to json, I have another PR pending to do, with changes I made after this PR, I haven't PR yet because It has conflicts, so the idea is to merge this PR and then the next one is where the jsonc will be applied and other stuff that I don't recall right now...

@dblythy
Copy link
Member

dblythy commented Jun 30, 2022

Right, think your changes might have got overriden when merging conflicts. Have a look at the “files changed” tab, the only changes in this PR are changing js to json.

We value passion and ideas - but let’s make sure to open an issue first and reference that issue in the PR. This helps all of us understand the problem, and what is being fixed. It seems that there are some great ideas in the commits here (that somehow were reset) so I’m looking forward to discussing them in their respective issues.

Plus, it is cool keeping track of how many issues and PRs you have, I am up to 232, only 400 behind @mtrezza 😂

@MrMartinR
Copy link
Contributor Author

checking this PR, you are right, only the changes to js to json.. 🤷‍♂️.. I am gonna delete this PR, check the Issues, and create the other PR with the Fix in the AWS deployment and the js to jsonc stuff..

@MrMartinR MrMartinR closed this Jun 30, 2022
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.

2 participants