Page MenuHomePhabricator

mediawiki/extensions/PageAssessments history should be cleaned and reimported + other concerns
Closed, ResolvedPublic

Description

Hello, I have a few concerns with the way mediawiki/extensions/PageAssessments.git has been created and is contributed to. Maybe I am too idealist but here are a few things that are worth fixing up:

It has created by importing the whole history of the BoilerPlate extension then lot of bits got removed. The history is misleading as a result. I would recommend to recreate it entirely:

  • craft an initial commit introducing the .gitreview file (with the proper repo) and force push it to get rid of the history
  • create another patch that introduce the basic structure and send it for review in Gerrit so at least we know the bases are proper.

Then get the development going on.

A second concern is that changes are being pushed directly to Gerrit bypassing review entirely. If the extension is to be deployed to production, the change have to be peer reviewed.

Finally we would need some basic CI on the repo. Most probably as part of the first commit proposing code. The entry points and basic commands are included in BoilerPlate but would still need to be tweaked and validated by CI.

The CI configuration itself is straightforward. A recent example is f634123123edb963ae31f0528d6bccf7fee147d4 for the extension EventBus. That would work for PageAssesment.

Feel free to poke #wikimedia-releng or myself on IRC (hashar , Europe).

Event Timeline

hashar raised the priority of this task from to Needs Triage.
hashar updated the task description. (Show Details)
hashar added subscribers: hashar, kaldari, Niharika.
DannyH set Security to None.
DannyH moved this task from New & TBD Tickets to Needs Discussion on the Community-Tech board.

@hashar: Definitely agree, just wasn't sure how to do that.

A second concern is that changes are being pushed directly to Gerrit bypassing review entirely. If the extension is to be deployed to production, the change have to be peer reviewed.

I was also surprised that doing the import from GitHub bypassed review entirely. @QChris, @greg: What's you're opinion on this. Right now importing a repo from GitHub to Gerrit bypasses code review entirely. It seems like we should change this so that imports are submitted for review as well.

@hashar: Whenever I try to force push the reset, I get the error:

! [remote rejected] master -> master (non-fast forward)

According to stackoverflow, this means that the server is configured to not allow non-fast forward pushes (via denyNonFastforwards = true). Any ideas how to work around that? If you can reset the repo yourself, please go ahead and do that. Or maybe @QChris can help.

In https://gerrit.wikimedia.org/r/#/admin/projects/mediawiki/extensions/PageAssessments,access I have granted force push right to member of Gerrit group extension-PageAssessments though they are all project owners already.

Niharika poked me about it on IRC. Work in progress :-)

@NiharikaKohli phased out the whole history in favor of a single change introducing .gitreview. Now a new patch can be proposed to introduce all the code. Thank you !!!

@NiharikaKohli: Feel free to go ahead and a submit a patch with the rest of the (pre-Job Queue) code.