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(paths): handle ../ in relative path check #18

Merged
merged 3 commits into from
Jun 24, 2022
Merged

fix(paths): handle ../ in relative path check #18

merged 3 commits into from
Jun 24, 2022

Conversation

JounQin
Copy link
Contributor

@JounQin JounQin commented Jun 24, 2022

close #17

@JounQin
Copy link
Contributor Author

JounQin commented Jun 24, 2022

@privatenumber Thanks for quick refactor, can we merge and release?

@privatenumber
Copy link
Owner

Thanks for the PR and the test-case.


I appreciate you reporting issues, but in the future, please:

  • Don't open issues with zero effort to communicate the problem

    Only providing links to dense CI reports or a massive repository and expecting others to understand or investigate it is unproductive and disrespectful of other peoples time.

    I had to clone a massive repository, only to realize the command you provided spawns many other commands, and I had to put in work to find out where the problem was, and why it was a problem. (This also happened with your other issue in tsx.)

    If you can simply open a PR with a failing test-case like you did here, that is enough for reproduction and will be the fastest way to get your changes merged.

  • Don't comment asking people for review or release

    Maintainers already get notified on PR and comments, so further comments that ask us to prioritize your issue can be annoying. Not to mention, you saw my commits so you could tell I was already on it. The only thing slowing this down is me having to write this because of your behavior on my repos.

@JounQin
Copy link
Contributor Author

JounQin commented Jun 24, 2022

Sorry about your feeling.

At first, I just didn't find what is causing the issue. Personally I also maintain a lot of OSS projects, so I understand reproduction is required, and I did provide, but the project is just huge to extract a minimal reproduction, I don't have idea what's the root cause, then I can't reduce the reproduction.

That's just an idea when the error occurred, and then I tried to read the compiled codes, and had an idea how it was caused, but I believe I can't be so lucky for times.

I apologize if you don't feel good on this, maybe I need to wait for seconds before posting the issue.

Finally, hope you happiness.

@privatenumber
Copy link
Owner

Thanks @JounQin

I understand it can be tough to investigate and produce a reproduction. But it's either you do it or I do it, and it's often possible that the problem isn't even related to the project. I encourage you to spend that time before opening an issue next time.

In the end, a clearly written issue with a reproduction saves everyone time. 👍

@privatenumber privatenumber changed the title fix: check ../ for relative paths fix(paths): handle ../ in relative path check Jun 24, 2022
@privatenumber privatenumber merged commit 96a5c10 into privatenumber:develop Jun 24, 2022
@JounQin JounQin deleted the fix/relative_paths branch June 24, 2022 20:32
@privatenumber
Copy link
Owner

🎉 This PR is included in version 4.0.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

@JounQin
Copy link
Contributor Author

JounQin commented Jun 24, 2022

I encourage you to spend that time before opening an issue next time.

Yeah, I was too anxious before posting the issue. I need to improve this for my whole life.

In the end, a clearly written issue with a reproduction saves everyone time. 👍

🍺

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

Successfully merging this pull request may close these issues.

Unexpected Error: Non-relative paths are not allowed when 'baseUrl' is not set. Did you forget a leading './'?
2 participants