Skip to content

Conversation

@Phobez
Copy link
Collaborator

@Phobez Phobez commented Sep 27, 2021

Added and changed feedback messages according to issue #3 and existing TODOs.

@Phobez Phobez requested a review from Aechylus September 27, 2021 07:34
@Phobez Phobez self-assigned this Sep 27, 2021
@Phobez Phobez linked an issue Sep 27, 2021 that may be closed by this pull request
11 tasks
Copy link
Collaborator

@Aechylus Aechylus left a comment

Choose a reason for hiding this comment

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

Non breaking change in general. Though, the more I think about it, the more I'm leaning towards having the messages in TMConstants as a static function instead; that way, we can specify the arguments to the messages explicitly, rather than relying on the %s in the string format. Plus, we can introduce logic in the String generation, i.e. calling helper function addErrorColor() or something like that.

@Phobez
Copy link
Collaborator Author

Phobez commented Oct 1, 2021

Should that be done as part of this PR or after this PR though?

@Phobez
Copy link
Collaborator Author

Phobez commented Oct 1, 2021

Posting this here from a WA chat for posterity.

[3:46 pm, 01/10/2021] Aechylus: We can make it a separate PR if you wanna make the history cleaner and more traceable
[3:47 pm, 01/10/2021] Phobez: Then I'll make it a separate PR with its own issue

Non breaking change in general. Though, the more I think about it, the more I'm leaning towards having the messages in TMConstants as a static function instead; that way, we can specify the arguments to the messages explicitly, rather than relying on the %s in the string format. Plus, we can introduce logic in the String generation, i.e. calling helper function addErrorColor() or something like that.

Therefore, this'll be a separate issue and PR.

@Phobez Phobez merged commit fe1059f into dev Oct 1, 2021
@Phobez Phobez deleted the command-feedback branch November 20, 2021 08:56
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.

Not enough command feedback.

3 participants