-
Notifications
You must be signed in to change notification settings - Fork 56
Change the default warning message of CmdletPreviewAttribute and allow setting ETA #395
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
Conversation
d7ac7f6 to
3d75c4a
Compare
d018d66 to
0adc6a0
Compare
| this._message = string.IsNullOrEmpty(message) ? Resources.PreviewCmdletMessage : message; | ||
| } | ||
|
|
||
| public CmdletPreviewAttribute(string message, string estimatedDateOfGa) : this(message) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we avoid confusion for dates like "1/2/2023"? Some may interpret it as Jan 2nd, some Feb 1st.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The date assume the date follows "en-US" culture like breaking change attribute, which means dates are written in the month–day–year order.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The benefits of using local culture to display estimated GA date is customer are more familiar with their local date. But considering:
- to keep consistent with breaking change attribute, CmdletPreviewAttribute won't display estimatedGaDate with local culture to avoid introduce new confusion.
- Customer may use the output as the input of DateTime, standard date time will avoid parse issue.
Uh oh!
There was an error while loading. Please reload this page.