-
Notifications
You must be signed in to change notification settings - Fork 20
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
refactor: Change the formatting of the community PR message to make it tighter #339
refactor: Change the formatting of the community PR message to make it tighter #339
Conversation
Thanks for the pull request, @xitij2000! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #339 +/- ##
=======================================
Coverage 89.72% 89.72%
=======================================
Files 38 38
Lines 3026 3026
Branches 229 229
=======================================
Hits 2715 2715
Misses 279 279
Partials 32 32 ☔ View full report in Codecov by Sentry. |
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.
Thanks @xitij2000, this looks great 👍
Hey @feanil, this is just a small follow-up for openedx/wg-coordination#147, would you mind giving it a quick look? |
87aaef2
to
0260442
Compare
@xitij2000 this needs a rebase but then I can merge and deploy it. |
5d95078
to
5291159
Compare
Not sure why the test are failing here. Doesn't seem to be related to the changes, but I'll have a closer look later. |
I've hopefully fixed the test issues. |
@xitij2000 I came here again today as part of my weekly OSPR management pass and saw that the build was green but the branch had fallen out of date with the base branch again. So to speed things up I figured I'd merge in the latest changes from master myself, wait for the build to pass, and ping Feanil for review again. Unfortunately the merge seems to have (re-)introduced a test failure. Sorry about that! Could you please have a look when you get a minute? |
81124d5
to
fb6e605
Compare
@itsjeyd No worries! I just needed to drop my commit with the temporary fix. |
… spacing The current message is too spaced out, this change fixes the spacing issues and moves the hr to the correct place.
fb6e605
to
9824556
Compare
@xitij2000 Ah OK 😅 That's great. @feanil We got a green build here now, could you please merge this PR? That'll wrap up openedx/wg-coordination#147. |
The update is now deployed. Thanks @xitij2000 |
The current message is too spaced out, this change fixes the spacing issues and moves the hr to the correct place.
The h3 and h4 have been removed since they add too much spacing around in the collapsable and make the message seem too spaced out. Being inside a collapsable section already highlights them quite a bit so I'm hoping this is okay.
Screenshot after update:
