feat: Update the community PR message #288

xitij2000 · 2024-04-24T17:05:41Z

This change updates the community PR message to provide better guidance and updated links to the latest documentation.

The message now also directs contributors to the correct team or person to tag to get their PR reviewed and merged.

openedx-webhooks · 2024-04-24T17:05:46Z

Thanks for the pull request, @xitij2000! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

supporting documentation
Open edX discussion forum threads
timeline information ("this must be merged by XX date", and why that is)
partner information ("this is a course on edx.org")
any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

itsjeyd · 2024-04-26T06:14:23Z

Addresses #287 by implementing new welcome message for OSPRs.

itsjeyd · 2024-04-26T06:15:07Z

FYI @mphilbrick211 @arbrandes

DanielVZ96 · 2024-04-29T03:17:37Z

openedx_webhooks/templates/github_community_pr_comment.md.j2

+{% if owner %}
+This repository is currently maintained by `@{{ owner }}`.  Tag them in a comment and let them know that your changes are ready for review.
+{%  else %}
+This repository is currently unmaintained.  To get help with finding a technical reviewer, tag the community contributions project manager for this PR in a comment and let them know that your changes are ready for review:


I think unmaintained leaves a feeling of not being developed, when the project may still be actively developed but without an owner yet.

Suggested change

This repository is currently unmaintained. To get help with finding a technical reviewer, tag the community contributions project manager for this PR in a comment and let them know that your changes are ready for review:

This repository currently has no owner. To get help with finding a technical reviewer, tag the community contributions project manager for this PR in a comment and let them know that your changes are ready for review:

I'll let @itsjeyd chime in on this since the text draft comes from him. I'm guessing the idea is that every maintained repo should have an owner.

@DanielVZ96 @xitij2000

I'm guessing the idea is that every maintained repo should have an owner.

Yes that's right.

Before the maintenance working group was created and started reforming the concept of maintainership earlier this year, things were a lot fuzzier -- there was the concept of (edX/2U) code ownership, which didn't really include the responsibilities that you'd expect a maintainer to take on, and a separate concept of maintainership (as defined in the context of the Maintainership Pilot program). For some repos, the two would overlap (i.e., edX/2U officially owned these repos and also signed on as a maintainer in the context of the pilot), and for others they didn't. That was creating confusion and contributing to delays in the OSPR review process.

Of late there's been a tendency to standardize on "maintainer" terminology, so I think I was trying to follow that here and avoid falling back on "code ownership" language.

... unmaintained leaves a feeling of not being developed, ...

I do agree with this, though. Since the goal is to eventually get to a point where all relevant repos are under active maintenance, maybe it would help to meet in the middle and say something like:

This repository has no maintainer (yet).

I think we can use more cues from the catalog file here. The lifecycle should indicate if the repo is in production, experimental or deprecated. For deprecated repos we can confidently say that the repo is unmaintained, whereas for a production repo we can use the above language. Does that sound better or should be keep things simple?

@xitij2000 Sounds good to me, more nuance would be great 👍 CC @mphilbrick211 ⬆️

DanielVZ96 · 2024-04-29T03:29:03Z

openedx_webhooks/templates/github_community_pr_comment.md.j2

-GitHub pull request interface. As a reminder,
-[our process documentation is here](http://edx-developer-guide.readthedocs.org/en/latest/process/overview.html).
-{% endfilter %}
+Thanks for the pull request, `@{{ user }}`!


why is the user in back ticks now? i see that it works ok without them right now.

I'm not entirely sure, but I've taken the contents from here.

Generally, the idea with putting it in backticks is to avoid actually pinging the user. However, in this case perhaps the user should be notified?

@itsjeyd Should the backticks be here? I think not notifying the maintainer group or user makes sense but the PR author perhaps should be OK to ping?

@xitij2000 @DanielVZ96 I don't remember why I added those backticks, feel free to remove them 😅

@xitij2000 @itsjeyd i also prefer the ping 😅

itsjeyd · 2024-05-23T14:56:22Z

Hey @xitij2000, could you have a look and see what's keeping the tests from passing here? Once the build is green we'll be able to move forward with engineering review. (Commenting as OSPR manager right now :))

xitij2000 · 2024-05-24T12:50:22Z

Hey @xitij2000, could you have a look and see what's keeping the tests from passing here? Once the build is green we'll be able to move forward with engineering review. (Commenting as OSPR manager right now :))

It's a known issue with rate limits when uploading coverage. The tests themselves have passed. I don't know if an admin running the tests will bypass this.

itsjeyd · 2024-05-24T15:30:38Z

@xitij2000 OK I see, we should be good to put this up for review then.

@feanil This is the PR that updates the OSPR welcome message that we discussed in the maintenance working group a while back. Would you be able to review it? Happy to ping someone else if you don't have bandwidth, please let us know.

CC @mphilbrick211

feanil · 2024-06-10T13:49:30Z

Hey, all. Sorry for the quite, I am planning on reviewing this, this week.

itsjeyd · 2024-06-13T07:07:46Z

Great, thanks @feanil.

feanil

@xitij2000 @itsjeyd thank you for being so soooo patient. The changes look great, I just had one question related to formatting and then I think this is good to land.

feanil · 2024-06-26T21:24:41Z

openedx_webhooks/templates/github_community_pr_comment.md.j2

@@ -1,52 +1,105 @@
-{% filter replace("\n", " ")|trim %}


Why was this removed? Do we believe that it's not necessary anymore?

That filter was being applied in sections where newlines were added in the original document for wrapping but were not supposed to be present in the output. In the new document the placement of newlines in significant and is supposed to be retained in the output.

feanil · 2024-06-26T21:32:13Z

Also, the coverage test failure should be resolved now if you rebase the PR.

This change updates the community PR message to provide better guidance and updated links to the latest documentation. The message now also directs contributors to the correct team or person to tag to get their PR reviewed and merged.

codecov · 2024-06-27T07:32:25Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.90%. Comparing base (a3d63a8) to head (6121962).
Report is 31 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #288      +/-   ##
==========================================
+ Coverage   89.76%   89.90%   +0.13%     
==========================================
  Files          38       38              
  Lines        2882     2921      +39     
  Branches      397      407      +10     
==========================================
+ Hits         2587     2626      +39     
  Misses        268      268              
  Partials       27       27

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

feanil

Awesome, I don't think I'll have time to ship the code today but I'll ship this onto the testing server as soon as I can.

feanil · 2024-07-17T13:52:45Z

Ok, changes have been tested here: openedx-unsupported/mdrst#40 and openedx-unsupported/mdrst#41

I'm merging and pushing this to production.

openedx-webhooks · 2024-07-17T13:52:53Z

@xitij2000 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

feanil · 2024-07-17T13:57:08Z

Change is deployed to production, thanks for all the hard work everyone, appreciate your patience as always!

itsjeyd · 2024-07-18T14:42:35Z

Happy to see this land, thanks @feanil!

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Apr 24, 2024

xitij2000 force-pushed the kshitij/update-community-pr-comment branch from 15d18ee to 2de654f Compare April 25, 2024 02:24

xitij2000 marked this pull request as ready for review April 25, 2024 12:08

itsjeyd added the core contributor PR author is a Core Contributor (who may or may not have write access to this repo). label Apr 26, 2024

itsjeyd requested a review from feanil April 26, 2024 06:09

DanielVZ96 reviewed Apr 29, 2024

View reviewed changes

DanielVZ96 approved these changes May 3, 2024

View reviewed changes

itsjeyd mentioned this pull request May 23, 2024

Update welcome message from OSPR bot with info and checklist for authors #287

Closed

3 tasks

itsjeyd added the waiting for eng review PR is ready for review. Review and merge it, or suggest changes. label May 24, 2024

feanil reviewed Jun 26, 2024

View reviewed changes

xitij2000 added 2 commits June 27, 2024 12:59

feat: Update the community PR message

7b2710d

This change updates the community PR message to provide better guidance and updated links to the latest documentation. The message now also directs contributors to the correct team or person to tag to get their PR reviewed and merged.

fixup!: Update message, add tests

6121962

xitij2000 force-pushed the kshitij/update-community-pr-comment branch from c99eebe to 6121962 Compare June 27, 2024 07:29

feanil approved these changes Jun 27, 2024

View reviewed changes

itsjeyd mentioned this pull request Jul 10, 2024

Core contributors as backup reviewers openedx/wg-coordination#104

Closed

feanil merged commit 29a532c into openedx:master Jul 17, 2024
6 checks passed

xitij2000 deleted the kshitij/update-community-pr-comment branch July 25, 2024 08:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Update the community PR message #288

feat: Update the community PR message #288

xitij2000 commented Apr 24, 2024

openedx-webhooks commented Apr 24, 2024 •

edited

Loading

itsjeyd commented Apr 26, 2024

itsjeyd commented Apr 26, 2024

DanielVZ96 Apr 29, 2024

xitij2000 Apr 29, 2024 •

edited

Loading

itsjeyd Apr 30, 2024 •

edited

Loading

xitij2000 Apr 30, 2024

itsjeyd Apr 30, 2024

DanielVZ96 Apr 29, 2024

xitij2000 Apr 29, 2024

itsjeyd Apr 30, 2024

DanielVZ96 May 2, 2024

itsjeyd commented May 23, 2024

xitij2000 commented May 24, 2024

itsjeyd commented May 24, 2024

feanil commented Jun 10, 2024

itsjeyd commented Jun 13, 2024

feanil left a comment

feanil Jun 26, 2024

xitij2000 Jun 27, 2024

feanil commented Jun 26, 2024

codecov bot commented Jun 27, 2024 •

edited

Loading

feanil left a comment

feanil commented Jul 17, 2024

openedx-webhooks commented Jul 17, 2024

feanil commented Jul 17, 2024

itsjeyd commented Jul 18, 2024

	This repository is currently unmaintained. To get help with finding a technical reviewer, tag the community contributions project manager for this PR in a comment and let them know that your changes are ready for review:
	This repository currently has no owner. To get help with finding a technical reviewer, tag the community contributions project manager for this PR in a comment and let them know that your changes are ready for review:

feat: Update the community PR message #288

feat: Update the community PR message #288

Conversation

xitij2000 commented Apr 24, 2024

openedx-webhooks commented Apr 24, 2024 • edited Loading

itsjeyd commented Apr 26, 2024

itsjeyd commented Apr 26, 2024

Choose a reason for hiding this comment

xitij2000 Apr 29, 2024 • edited Loading

Choose a reason for hiding this comment

itsjeyd Apr 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

itsjeyd commented May 23, 2024

xitij2000 commented May 24, 2024

itsjeyd commented May 24, 2024

feanil commented Jun 10, 2024

itsjeyd commented Jun 13, 2024

feanil left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feanil commented Jun 26, 2024

codecov bot commented Jun 27, 2024 • edited Loading

Codecov Report

feanil left a comment

Choose a reason for hiding this comment

feanil commented Jul 17, 2024

openedx-webhooks commented Jul 17, 2024

feanil commented Jul 17, 2024

itsjeyd commented Jul 18, 2024

openedx-webhooks commented Apr 24, 2024 •

edited

Loading

xitij2000 Apr 29, 2024 •

edited

Loading

itsjeyd Apr 30, 2024 •

edited

Loading

codecov bot commented Jun 27, 2024 •

edited

Loading