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

New constants about tools #43

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
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

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
Copy link
Collaborator Author

Land before #44

@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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants