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

Formatted README #77

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Formatted README #77

wants to merge 12 commits into from

Conversation

PhoneDroid
Copy link

WHY are these changes introduced?

Makes the README more readable / useful.

WHAT is this pull request doing?

  • Corrected package ( GitHub ) links
  • Added NPM links
  • General formatting
  • Made package names copyable
  • Switched list item headings to the topic of the item

@PhoneDroid PhoneDroid requested a review from a team as a code owner December 27, 2022 15:51
@PhoneDroid
Copy link
Author

Please add a description and tags to the repository and disable the unsed Projects tab.

@lizkenyon lizkenyon closed this Dec 5, 2023
@PhoneDroid
Copy link
Author

Wow, no response for a year & then closed without reason, thanks.

@lizkenyon
Copy link
Contributor

lizkenyon commented Dec 5, 2023

Hi @PhoneDroid

There was some formatting additions you added that we did like. But we were not ready to accept all the changes. We closed because the PR was quite old.

If you would like to open a new PR or update this one with with adding the badge links for the packages we could look at getting that merged.

And please make sure to sign the CLA!

We look forward to your contribution! 😄

@lizkenyon lizkenyon reopened this Dec 5, 2023
@PhoneDroid
Copy link
Author

@lizkenyon I reworked the content that was added since 2022.
https://github.com/PhoneDroid/Contribute-Shopify_App_JS/tree/main

.editorconfig Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@PhoneDroid PhoneDroid mentioned this pull request Dec 6, 2023
README.md Outdated Show resolved Hide resolved
This mono-repo supports the following packages:
# Shopify App JS

*A mono-repo containing a collection of packages*
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a particular reason why you are limiting the line length so much?

Copy link
Author

Choose a reason for hiding this comment

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

Because humans can't properly comprehend lines longer than 60~ characters

Copy link
Contributor

Choose a reason for hiding this comment

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

For now I would prefer we don't use the 60 character limit, because the rest of our repositories do not follow this rule.

#### [`@shopify/shopify-app-remix`](./packages/shopify-app-remix)
<br>

- ### **[Remix]**
Copy link
Contributor

Choose a reason for hiding this comment

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

These can just be H3, remove the bullets points. And same comment for the rest of the headings.

Copy link
Author

Choose a reason for hiding this comment

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

It's a stylistic choice to give a visually distinct look for the package listings, they are lists of items after all, not topics ..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants