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

fix: npm readme #136

Merged
merged 5 commits into from
Oct 27, 2023
Merged

fix: npm readme #136

merged 5 commits into from
Oct 27, 2023

Conversation

Pehesi97
Copy link
Contributor

@Pehesi97 Pehesi97 commented Oct 25, 2023

This PR creates a symbolic link to README.md in the npm folder so the same readme is displayed in GitHub and NPM.

To test: clone this branch and see if the npm/README.md file is populated. We also will need to check the next release to see if the README will be populated in the NPM website.

Closes #137

@Pehesi97 Pehesi97 requested a review from a team October 25, 2023 00:22
@Pehesi97 Pehesi97 self-assigned this Oct 25, 2023
@francardoso93
Copy link
Contributor

Even though this is just a symbolic link, do we need to have the same file content twice in the repo?
A possible better approach would be creating this file/link dynamically in the workflow before releasing

@Pehesi97
Copy link
Contributor Author

That would work for me too. I'll work on that.

@Pehesi97
Copy link
Contributor Author

@francardoso93 changes made

@@ -65,6 +65,7 @@ jobs:
run: |
cd npm
./publish.sh
ln ../README.md README.md
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to run before the publish.sh script

Copy link
Contributor

@francardoso93 francardoso93 Oct 27, 2023

Choose a reason for hiding this comment

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

Also, you need to remove the existing README.md to avoid conflict

Copy link
Contributor Author

Choose a reason for hiding this comment

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

true.

@francardoso93
Copy link
Contributor

Currently the symlinks are being created both on commited files AND dynamically through workflow. Now they should go with only one of these approaches

@Pehesi97
Copy link
Contributor Author

@francardoso93 changes made.

Copy link
Contributor

@francardoso93 francardoso93 left a comment

Choose a reason for hiding this comment

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

LGTM

@Pehesi97 Pehesi97 merged commit 7cfac95 into main Oct 27, 2023
@Pehesi97 Pehesi97 deleted the npm-readme branch October 27, 2023 19:37
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.

Work on the readme for the initium-cli package in NPM
2 participants