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

Change the error messages in relpath function. #47450

Merged
merged 1 commit into from
Nov 8, 2022
Merged

Change the error messages in relpath function. #47450

merged 1 commit into from
Nov 8, 2022

Conversation

the-lazy-learner
Copy link
Contributor

@the-lazy-learner the-lazy-learner commented Nov 4, 2022 •
edited
Loading

This was an attempt to fix issue #47443
I'm just a beginner, so please guide me in the right direction. I thought isempty checked for empty string arguments, and that an unspecified argument would rather cause a compile-time error. Looking forward to feedback

the-lazy-learner
the-lazy-learner commented Nov 4, 2022
View reviewed changes
Copy link
Contributor Author

@the-lazy-learner the-lazy-learner left a comment

Choose a reason for hiding this comment

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

This was an attempt to fix issue #47443
I'm just a beginner, so please guide me in the right direction. I thought isempty checked for empty string arguments, and that an unspecified argument would rather cause a compile-time error. Looking forward to feedback

@KristofferC KristofferC added the error messages Better, more actionable error messages label Nov 4, 2022
@KristofferC
Copy link
Sponsor Member

KristofferC commented Nov 8, 2022 •
edited
Loading

Thanks for the PR. This is indeed an improvement!

@IanButterworth IanButterworth changed the title Change the error messages in relpath function. Change the error messages in relpath function Nov 8, 2022
@IanButterworth IanButterworth changed the title Change the error messages in relpath function Change the error messages in relpath function. Nov 8, 2022
@StefanKarpinski
Copy link
Sponsor Member

Any reason why there's no CI checks on this PR?

@giordano
Copy link
Contributor

giordano commented Nov 8, 2022

There is some problems on GitHub's side with this PR, we can't merge it.

@the-lazy-learner
Copy link
Contributor Author

the-lazy-learner commented Nov 8, 2022 •
edited
Loading

@giordano Could you please let me know what things need to be fixed for the tests to pass?

@giordano
Copy link
Contributor

giordano commented Nov 8, 2022

Can you please open a new PR?

@the-lazy-learner
Copy link
Contributor Author

Okay will do in a while

@giordano giordano changed the base branch from master to KristofferC-patch-1 November 8, 2022 17:08
@giordano giordano requested review from DilumAluthge and a team as code owners November 8, 2022 17:08
@giordano giordano changed the base branch from KristofferC-patch-1 to master November 8, 2022 17:09
@giordano giordano removed request for a team and DilumAluthge November 8, 2022 17:09
@giordano giordano merged commit cf9cac2 into JuliaLang:master Nov 8, 2022
@giordano giordano mentioned this pull request Nov 8, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
error messages Better, more actionable error messages
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@the-lazy-learner @KristofferC @StefanKarpinski @giordano