-
Notifications
You must be signed in to change notification settings - Fork 594
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
[rush] Initial implementation of Rush alerts feature #4801
Merged
Merged
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
g-chao
requested review from
iclanton,
octogonz,
apostolisms,
D4N14L and
dmichon-msft
as code owners
June 23, 2024 23:19
g-chao
force-pushed
the
chao/rush-notification
branch
from
July 1, 2024 20:39
55ce0bf
to
9cce646
Compare
…hema" field is present; fix an issue where loading errors were silently discarded
…ngs were omitted in rush-alerts.json
… array of strings
octogonz
approved these changes
Jul 17, 2024
octogonz
changed the title
[rush] Prototype for Rush alerts feature
[rush] Initial implementation of Rush alerts feature
Jul 17, 2024
docs here: microsoft/rushstack-websites#261 |
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.
Summary
A prototype for Rush alerts feature. Learn more about background of this feature: #4782
In this PR, we presents a MVP for this feature, and explains the idea, flow and how end users can use this feature.
Any feedback is welcome here.
Details
MVP demo
I created a sample repo to demo this feature. To try the MVP demo:
rush-alerts-demo
branch3.1 Run rush commands first time, nothing showing in the CLI.
3.2 Run rush commands afterwards, it will print out alerts like below:
Flow explanation
To use this feature, users need to turn on
enablePushRushAlertsToEndUsersInCLI
inexperiments.json
.Generally there are two stages in this feature:
For step 1, we mentioned several ways to achieve it in the design proposal #4782. In this MVP, we chose to store alerts config in a Git branch to prove the idea.
From users perspective, they need to config the alerts data under
common/config/rush/alerts-config.json
.Here is an example of
alerts-config.json
:Now, when rush engine execute any commands, it will read this config and process the logic there and output the final
alerts-state.json
undercommon/temp
folder.Here is an example of
alerts-state.json
:You might notice that
alerts-config.json
andalerts-state.json
are pretty similar, however, their purpose is different.alerts-config.json
is intent to be used by users to config the rush alerts feature behavior and supply the alerts data.alerts-state.json
is used by rush internally to keep track the state of rush alerts, also to support more advance features in the later milestones.For step 2, whenever execute any rush commands, if there is data in `alerts-state.json, print out the alerts in CLI.
Future milestones
How it was tested
Not yet implemented
Impacted documentation
To be continued