Problem/Motivation

Steps to reproduce

Proposed resolution

Remaining tasks

  • ✅ File an issue about this project
  • ☐ Manual Testing
  • ☐ Code Review
  • ☐ Accessibility Review
  • ☐ Automated tests needed/written?

Issue fork project_browser-3292562

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

bnjmnm created an issue. See original summary.

bnjmnm opened merge request !174

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented

Moved the fixture generator to a Drush command. The command also adds an update_n() hook to project_browser.install, so all that is needed to create a fixture update branch is run drush update:generate-fixture, wait for a little over an hour (the file generation seems to take longer than I remember...), and commit this changes.

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Status: Active » Needs review
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Assigned: Unassigned » chrisfromredfin
Status: Needs review » Reviewed & tested by the community

This looks great. Especially the generating of the hook_update_N.

Assigning to Chris for now, but will commit early next week if there aren't any concerns.

narendrar’s picture

narendrar
He/Him
Primary language English
Location Jaipur, India
commented

Added some suggestions.

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Issue tags: +Project Browser MVP

Adding MVP tag as this was specifically mentioned as something that should be in the alpha release.

I'm inclined to not implement the MR suggestions. They're good ones, but this is code that we know for sure is going away as it's all related to the fixture generation functionality which will be removed once the D.O. endpoint is available and can't be in the module when it's proposed for core inclusion.

chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented

OK I also reviewed this and I think it's fine, especially given its nature as a throwaway. "It works," so let's just get it in.

I don't have time to do the commit, but if Tim or Ben want to commit, you have my endorsement. :-p

  • tim.plunkett committed d0d4156 on 1.0.x authored by bnjmnm 
    Issue #3292562 by bnjmnm, tim.plunkett, narendraR, chrisfromredfin: Make...
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Assigned: chrisfromredfin » Unassigned
Status: Reviewed & tested by the community » Fixed

Awesome work @bnjmnm, thanks for the reviews @narendraR and @chrisfromredfin. Merged!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.