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

README is broken on NPM #277

Closed
kettanaito opened this issue Dec 30, 2019 · 10 comments · Fixed by #278
Closed

README is broken on NPM #277

kettanaito opened this issue Dec 30, 2019 · 10 comments · Fixed by #278
Assignees
Labels
bug DX This issue improves developer's experience good first issue Good place to start for newcomers

Comments

@kettanaito
Copy link
Owner

What:

NPM states that atomic-layout package doesn't have a README.

Why:

Due to the monorepo structure.

Environment:

  • atomic-layout version: 0.12.3

Steps to reproduce:

Steps to reproduce the behavior:

  1. Go to https://www.npmjs.com/package/atomic-layout
  2. See the following message instead of README:
"Unable to find a readme for atomic-layout@0.12.3"

Expected behavior:

atomic-layout package on NPM displays the current README from the repository.

Screenshots:

Screen Shot 2019-12-30 at 12 57 48

@kettanaito kettanaito added bug DX This issue improves developer's experience good first issue Good place to start for newcomers labels Dec 30, 2019
@ruhaise
Copy link
Collaborator

ruhaise commented Dec 31, 2019

@kettanaito i think to solve this we should have a copy of README.md in https://github.com/kettanaito/atomic-layout/tree/master/packages/atomic-layout as well, is it correct ?

@kettanaito
Copy link
Owner Author

Perhaps, let's see if we can achieve that with symlinks? NPM should support that.

@ruhaise ruhaise mentioned this issue Jan 1, 2020
5 tasks
@ruhaise ruhaise self-assigned this Jan 1, 2020
@ruhaise
Copy link
Collaborator

ruhaise commented Jan 1, 2020

@kettanaito this seems fixed the issue, but I'm still not sure if this is the correct approach?
now the README.md is moved to the packages/atomic-layout then i have added a symlink from it to the root of repository just for GitHub, now the npm will find theREADME.md from the package

@kettanaito
Copy link
Owner Author

@ruhaise We need to make sure the root-level README is not broken on GitHub. I think we should preserve the root-level and symlink it to packages/atomic-layout. Would that work in your opinion?

@ruhaise
Copy link
Collaborator

ruhaise commented Jan 1, 2020

@kettanaito that should work as well, atleast in github im sure, but im not sure about npm, i did some research and was not able to find any proper documentaion about it

@ruhaise
Copy link
Collaborator

ruhaise commented Jan 1, 2020

@kettanaito I have pushed the changes as you proposed, but i have no clue how to test this on NPM without publishing

@kettanaito
Copy link
Owner Author

Thanks for the update! Looks much better.

If you open the packages/atomic-layout you can see that the symlinked README has broken images. That is because the paths to the assets are relative to the root directory. This one is an interesting to solve. I'd suggest looking to other monorepo packages and how they handle README in their repository.

@ruhaise
Copy link
Collaborator

ruhaise commented Jan 1, 2020

@kettanaito I made the broken links from relative to absolute this seems fixed the issue, but I'm not sure if this is the right way. did a bit of research and couldn't find a better solution.

@ruhaise
Copy link
Collaborator

ruhaise commented Feb 18, 2020

@kettanaito what is the status of this issue? seems like the PR fixed it right?

@kettanaito
Copy link
Owner Author

@ruhaise, I believe so. Sorry, I've been busy with other tasks and haven't looked at your pull request in the full extent. Will do so in the nearest future. Thank you once more for contributing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug DX This issue improves developer's experience good first issue Good place to start for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants