Skip to content

New constants about tools #43

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
Merged
merged 1 commit into from
Feb 16, 2020
Merged

New constants about tools #43

merged 1 commit into from
Feb 16, 2020

Conversation

alshopov
Copy link
Collaborator

Bump version

@alshopov alshopov requested a review from rfindler February 16, 2020 14:10
@alshopov alshopov mentioned this pull request Feb 16, 2020
Merged
@alshopov
Copy link
Collaborator Author

Land before racket/drracket#346

@rfindler
Copy link
Member

I think that the tool-tool-phase string constant corresponds to an error in the tool that the developer should be fixing, not something that should show up to the actual user of DrRacket (or, put another way, there are many many more string constants that we would need to add if we wanted to have ones with the intended audience of that message). In contrast, the tool-error-phase1 message corresponds to a situation that is more of a generic catchall "something went wrong" that plausibly the end user would see. (That said, I wouldn't mind removing the tool-error-phase1 errors.)

... do you agree?

@alshopov
Copy link
Collaborator Author

I have no firm opinion on this one. Will you be OK if I just revert tool-tool-phase message to just a constant - as it was before? I will also update the string-constants package pull request.

@rfindler
Copy link
Member

rfindler commented Feb 16, 2020 via email

@alshopov
New constants about tools
06a8254 
Bump version

Signed-off-by: Alexander Shopov <[email protected]>
@alshopov
Copy link
Collaborator Author

That sound simplest...
Done. The corresponding PR racket/drracket#346 has also been updated and both are ready to land

@alshopov alshopov mentioned this pull request Feb 16, 2020
Open
@alshopov
Copy link
Collaborator Author

Land before #44

@alshopov alshopov mentioned this pull request Feb 16, 2020
Open
@rfindler rfindler merged commit 15ea530 into racket:master Feb 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rfindler rfindler Awaiting requested review from rfindler

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@alshopov @rfindler