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

Fixes for dots in paths and relative paths #108

Merged
merged 3 commits into from
Feb 7, 2024
Merged

Fixes for dots in paths and relative paths #108

merged 3 commits into from
Feb 7, 2024

Conversation

dividedmind
Copy link
Collaborator

@dividedmind dividedmind commented Jan 30, 2024 •
edited
Loading

Fixes #104
Fixes #100

@dustinbyrne could you please verify this fixes your issues?

@dividedmind dividedmind marked this pull request as ready for review January 30, 2024 19:29
@dividedmind dividedmind changed the title fix: Correctly handle dots in paths fix: Correctly handle dots in paths and use relative paths Jan 31, 2024
dustinbyrne
dustinbyrne requested changes Feb 2, 2024
View reviewed changes
Copy link
Contributor

@dustinbyrne dustinbyrne left a comment

Choose a reason for hiding this comment

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

It's close, but I'm still getting relative paths in some cases. See the package names and path displayed in the attached image.
image

Including the AppMap here as well.
2024-02-02T19:36:03.267Z_-v1-ai-speak.appmap.zip

@dividedmind
Copy link
Collaborator Author

dividedmind commented Feb 2, 2024 •
edited
Loading

It's close, but I'm still getting relative paths in some cases. See the package names and path displayed in the attached image. image

Including the AppMap here as well. 2024-02-02T19:36:03.267Z_-v1-ai-speak.appmap.zip

I'm going to need some help to reproduce this.

Edit: I took a look at the appmap; it's super weird. I want to understand better what's happening there and I'd love to reproduce it. (BTW, where are you running what from, where are your appmap.ymls and package.jsons?)

@dustinbyrne
Copy link
Contributor

Here's a basic reproduction of the issue. See the README for running instructions.
https://github.com/dustinbyrne/appmap-node-108-repl

@dividedmind dividedmind changed the title fix: Correctly handle dots in paths and use relative paths Fixes for dots in paths and relative paths Feb 6, 2024
@dividedmind
Copy link
Collaborator Author

@dustinbyrne should work now, can you take a look? thanks!

dustinbyrne
dustinbyrne approved these changes Feb 7, 2024
View reviewed changes
Copy link
Contributor

@dustinbyrne dustinbyrne left a comment

Choose a reason for hiding this comment

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

Looks good!

@dustinbyrne dustinbyrne merged commit a35df7e into main Feb 7, 2024
6 checks passed
@dustinbyrne dustinbyrne deleted the fix/package-names branch February 7, 2024 02:15
@appland-release
Copy link

🎉 This PR is included in version 2.15.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dustinbyrne dustinbyrne dustinbyrne approved these changes

@zermelo-wisen zermelo-wisen Awaiting requested review from zermelo-wisen

@kgilpin kgilpin Awaiting requested review from kgilpin

@apotterri apotterri Awaiting requested review from apotterri

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Package recorded with no name Use root-relative paths in AppMaps
3 participants
@dividedmind @dustinbyrne @appland-release