Theme review cycle and acceptance criteria

I was reviewing the recent pull requests in the theme repo today and was wondering what the criteria for reviewing and accepting a theme is?

I see a recent merge of a theme added 3 days ago. But there are dozens of theme submission pull requests that haven’t been reviewed for months. I recently submitted my first theme and I’m wondering if / when it will get added to the showcase.

Are the pending pull requests somehow insufficient to be showcased on the official themes site? If so, why? Why did this particular theme take precedence over the ones laying dormant for months?

I understand the maintainers are busy but it’s not exactly welcoming for theme creators when you contribute to the ecosystem and it gets completely ignored.

Please don’t take this the wrong way. I’m simply curious why so many themes are pending review. Some of them may be great themes that no one will know about if they only search on the theme showcase.

Thank you for your help.

1 Like

Bumping this

Hi @bep and @jmooring. Any thoughts on how the theme review / pull request merge process can be improved, even if that means soliciting volunteers to help?

I would be happy to participate myself. I’m just unsure of what the review process is, how much knowledge is needed to approve themes, or how to get added as a contributor to the theme site repo.

1 Like

I appreciate your positive spin on your thread.

We currently have 2 problems in this area:

  1. A big backlog of unmerged changes. Which, when time goes, means that the PRs needs to be rebased against the main branch before they can be merged.
  2. We should make the approval process a little more lenient. If the theme doesn’t obviously violate any terms, we merge it (which means looking at the PR and see that it has the vital info and image and a valid open source license). We can add more automatic stuff that helps remove themes that does not … behave.

As to the first, I have thought about some script to do the rebase, but haven’t gotten to it. The pragmatic solution would be to close all current PRs once we have the manpower to merge new ones, and ask people to open new ones.

If you could give me your GitHub user, I will add you.

And again, you reaching out about this is much appreciated.

2 Likes

Sounds great. My Github username is wjh18, same as on here.

Done.

And again, for the older theme submissions that needs rebasing I suspect the short term solution would be to, for the people who does not understand the concept of Git rebase, to create a new PR.

What are your thoughts on accepting merge commits and squashing the commits as opposed to requiring a rebase? It looks like the repo settings already allow that, and I know there are a few PRs that have already fixed merge conflicts this way (my own included).

That way anyone with a merge conflict who doesn’t know how to rebase can still use their existing PR. And it will put less pressure on reviewers to merge asap to avoid having to ask the PR submitter to manually rebase.

I know it’s not ideal for keeping a linear history but it could make the workflow easier. I can open a discussion in the theme team if you’d prefer to discuss there.

3 Likes

+1 to this thread, and thanks for raising this @wjh18 (and tagging me on my Theme PR!)

@bep - Would it be useful to have additional folks looking at this (e.g. contributing into Hugo by reviewing themes, etc.)? Happy to look into this / be a part, if this could be useful.

1 Like

Yes, any merge type is great as long as it gets the job done (a mostly care about the commit history on the main Hugo code repo).

1 Like

Excellent. I’ll start working through some of the older PRs here soon then if that’s cool w/ everyone.

I appreciate that @chrisreddington, the more help we can get the better. I unfortunately don’t have access to add you as a contributor to the theme repo so I’ll defer to @bep on that.

3 Likes

Hey @wjh18 and @bep - just wanted to check-in if there was any progress on this? Happy to lend a hand where I can to bring down some of the backlog here. Thanks!

Sorry, I got caught up for a few weeks with other things and was promptly removed as a contributor without warning.

No problem at all @wjh18. Hope that all’s ok! And sorry to hear of the removal of access.

@bep - There are some PRs that have been sat since November (mine since February), and there are some PRs that got merged 9 days ago, (raised on the same day).

I’m more than happy to wait in line for a review, but there’s no clear review cycle in place - which has made me de-prioritise any further work in developing my theme for re-use by others.

Are there any plans to build out a separate sub-team for Theme Reviews, to help keep expanding the themes available in the gallery / community that supports it? As before, happy to put my hat in the ring.