batches: display rollout windows configuration in user settings#50479
Conversation
|
Codenotify: Notifying subscribers in CODENOTIFY files for diff 5072054...60071fa.
|
Bundle size report 📦
Look at the Statoscope report for a full comparison between the commits 60071fa and 5072054 or learn more. Open explanation
|
ec3ee2c to
7173dd9
Compare
courier-new
left a comment
There was a problem hiding this comment.
I love the design you came up with for this! 😄 Left several comments but they're all pretty minor so here's a ✅ to go along with them.
c8222f8 to
0585d61
Compare
courier-new
left a comment
There was a problem hiding this comment.
This looks great; the refactor to use the site query makes a lot of sense to me, too. Left a couple nits, but really great work here! 🙌
Once this is merged, can we safely revert https://github.com/sourcegraph/sourcegraph/pull/50471, too?
There was a problem hiding this comment.
Nit: Could we give this its own file, like we do with UserCodeHostConnections?
There was a problem hiding this comment.
Sorry if this is glaringly obvious, but why is "every other day" the default?
There was a problem hiding this comment.
Oh no, it's okay; the statement is technically wrong. I'll reword it.
* If days are provided, join them with commas and capitalize each day's name.
* Otherwise, returns 'every other day'.There was a problem hiding this comment.
Can you tell, do we actually validate if you provide a number other than 0? I don't remember if I've tried. I wonder if it's worth actually assuming that we haven't validated that here (so only returning "None" if it's actually rate: 0), and showing "invalid configuration" with the little warning indicator if we get a bad rate like rate: 7 or rate: 200/24 hours.
This doesn't need to happen in this PR; we could log that as an enhancement. Just something that came to mind that could also be useful here.
There was a problem hiding this comment.
We do have a validation of the rate field when it's added. I've added screenshots above.
Not yet. I have one more PR to remove the JSContext field for rollout configuration in the Bulk Actions dropdown view. |
5200636 to
52d727b
Compare
Co-authored-by: Kelli Rockwell <kelli@sourcegraph.com>
…sArea.tsx Co-authored-by: Kelli Rockwell <kelli@sourcegraph.com>
…#51560) Make the web application bundle independent from the `//client/web/src/integration` and `//client/shared/src/testing`. It significantly slows down the Percy changes I iterate on locally. Previously they were connected via multiple mock files and the shared `isHotReloadEnabled` variable. The variable is redundant now, and the mock files are correctly moved to the `web_tests` target out of `web_lib`.
Co-authored-by: Kelli Rockwell <kelli@sourcegraph.com>
713faa7 to
60071fa
Compare
When rollout windows are configured on a Sourcegraph instance, there's no way for non-site admins to know the window that has been configured. We are displaying this window (if configured), in the
Batch Changessection of the UserSettings area.Test plan
Test 1
/users/<username>/settings/batch-changeson a Sourcegraph instance without rollout windows configured.Test 2
/users/<username>/settings/batch-changeson a Sourcegraph instance with rollout windows configured.App preview:
Check out the client app preview documentation to learn more.