Make Notification and Make Mail Consistency #54211
Closed
+49
−51
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1- Replacing `select` with `confirm`. 2- Modify the `getView()` method for consistency with `NotificationMakeCommand`. 3- Place the `getOptions()` method to the end of the class for consistency with `NotificationMakeCommand`.
1- No need to ask about the markdown view name, use naming conventions to guess it. 2- Added the `getView()` method for consistency with `MailMakeCommand`.
1- Remove `testItCanGenerateMailWithViewWithNoInitialInput()` method. 2- make some adjustments to the `testItCanGenerateMailWithNoInitialInput()` and `testItCanGenerateMailWithMarkdownViewWithNoInitialInput()` methods
1- Eliminating the need to ask about the markdown view name in `testItCanGenerateNotificationFileWithMarkdownTemplateWithNotInitialInput()` method
|
I think the only thing I would change is adding a suggested name for the make:mail Markdown view and no other changes in this PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There is no consistency between
make:notificationandmake:mailwithout arguments.make:notification asks about:
1- The name of the notification.
2- Whether to create a markdown view or not.
3- What is the name of the view.
While make:mail asks about:
1- The name of the mail.
2- Whether to create a view or an empty view or not.
I have noticed a few things:
1- There is no need to ask about the name of the view while creating the
notificationbecause our command should use naming conventions to guess it.2- There is no need to ask about creating an empty view while creating the
mail, It should only ask about whether to create a markdown view or not.3- getOptions() method used in both of them but in different places in the classes.
4- getView() method is written in one class but the other uses it's content inside another method.
These things are important for consistency.
According to PR #52355 and #52057