Skip to content

Proposed patch for issue 3353 #3360

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

manwar
Copy link
Contributor

@manwar manwar commented May 23, 2025

Hi @oalders,

Please review when you get time.
Issue: #3353

1) Remove author from contributors list
2) Use plural form only if more than one contributors

Many Thanks.

Best Regards,
Mohammad

    1) Remove author from contributors list
    2) Use plural form only if more than one contributors
@manwar
Copy link
Contributor Author

manwar commented May 23, 2025

This is how you see the same page as reported in the issue now:

image

@oalders
Copy link
Member

oalders commented May 23, 2025

Thanks @manwar! It looks like this needs the latest perltidy to be run on it.

@manwar
Copy link
Contributor Author

manwar commented May 23, 2025

And Moo looks like this:

image

@manwar
Copy link
Contributor Author

manwar commented May 23, 2025

Thanks @manwar! It looks like this needs the latest perltidy to be run on it.

Oops, will do it now.

Copy link

codecov bot commented May 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.52%. Comparing base (dd7274a) to head (480bfe1).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3360      +/-   ##
==========================================
+ Coverage   73.41%   73.52%   +0.10%     
==========================================
  Files          68       68              
  Lines        2389     2395       +6     
  Branches      335      336       +1     
==========================================
+ Hits         1754     1761       +7     
+ Misses        508      507       -1     
  Partials      127      127              
Files with missing lines Coverage Δ
lib/MetaCPAN/Web/Controller/Release.pm 90.74% <100.00%> (+0.54%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@manwar
Copy link
Contributor Author

manwar commented May 23, 2025

Finally Moose look like this:

image

Copy link
Member

@haarg haarg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could the commits be updated to include a summary of the change in the subject rather than the issue number? When reading through the commit log, issue numbers on their own aren't particularly useful.

}
}
$data->{contributors} = $contributors;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only applies to the release page. The Pod view would still include the extra contributors.

This filtering should probably happen in lib/MetaCPAN/Web/Model/API/Contributors.pm instead.

@@ -1,7 +1,7 @@
<div class="nav-header">Authored by: <a href="/author/[% $release.author %]" class="cpan-author">[% $release.author %]</a></div>
%% if $contributors.size() {
<div>
<button class="contributors-show-button btn-link">and [% $contributors.size() %] contributors</button>
<button class="contributors-show-button btn-link">and [% $contributors.size() %] [% $contributors.size() > 1 ? 'contributors' : 'contributor' %]</button>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be done with the pluralize function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants