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.

Sign up for GitHub

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

Jump to bottom

fix: updated docs #759

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
+5 −1
Open

fix: updated docs #759

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
92c12ff
fix: updated docs
Oct 15, 2024
51a8fdd
Merge branch 'master' into update-docs
ianrwhitney Oct 25, 2024
70a19ed
feat: respond to comments
ianrwhitney Oct 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,11 @@ build:

```
attach sistent to your project:
npm install [path to sistent repo ]
npm install [path to sistent repo]

alternative:
npm pack (in sistent repo)
npm install /path/to/file.tgz (in another repo)
Copy link
Member

Choose a reason for hiding this comment

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

why you changing this path to specific ?

Copy link
Author

Choose a reason for hiding this comment

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

i wasnt able to get sistent to attach when running meshery with the given commands in the readme. I had to pack in a tgz file and then ask npm to install that. This was the only way it was able to attach. If the orginial works for you maybe i can add this as an alternative here? Let me know your thoughts. thanks for your time to comment.

Copy link
Member

Choose a reason for hiding this comment

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

@nebula-aac, what do you think?

Copy link
Member

Choose a reason for hiding this comment

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

@SAHU-01 you might have a perspective to offer.

Copy link

@SAHU-01 SAHU-01 Oct 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@ianrwhitney if you wish to attach sistent with meshery, when you go into the ui folder of meshery and do a npm i [sistent path] it adds the sistent changes . If you attempt to attach sistent by executing npm i [sistent path] directly in meshery remote repo, it most likely doesn't work (at least in my case).

For the other repositories that I've added sistent code to, the npm i [sistent path] works well without an issue. So I believe @leecalcote we should keep the original one and maybe add a special case for meshery repo below, to either run it inside ui or to use a specific path like your suggestion @ianrwhitney.

Copy link
Author

Choose a reason for hiding this comment

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

yea i can add this as an alternative. thanks for the comments

Copy link
Contributor

Choose a reason for hiding this comment

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

It is okay as an alternative, but to prevent further confusion, it is better to raise the issue of what happens when you attempts to use npm install [sistent path] and we can help to do a work around. I haven't ran into this issue, and would like to investigate this if there is a step that may be missing.

```

> [!NOTE]
Expand Down
Loading