Moving into a Drupal CMS stable release, the DA needs to know that our contract is as minimal as possible for the Project object. We reviewed it (fjgarlin, hestenet, phenaproxima, drumm, chrisfromredfin) and have the following suggestions:
Drupal\Core\Url object, or null. Due to the way the Svelte code works, this also necessitates simplifying $images so that they're all arrays with only 'file' (a Url object) and 'alt' elements.| Comment | File | Size | Author |
|---|---|---|---|
| #15 | bad-logos.png | 698.71 KB | chrisfromredfin |
| #7 | Screenshot 2025-02-07 at 15.02.10.png | 63.98 KB | fjgarlin |
Show commandsStart 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:
changes
Comments
Comment #2
phenaproximaNot a stable blocker for Drupal CMS.
Comment #3
phenaproximaThis also need not be on our radar for a general release of Drupal CMS. Re-categorizing as a beta blocker because the Project class is API, which can change in alpha releases but not in beta.
Comment #4
phenaproximaComment #5
phenaproximaComment #6
phenaproxima$warnings is used extensively in the Svelte code, but the backend is a different story. The only source plugin which uses it is ProjectBrowserTestMock. The fact that it's not used by the DrupalOrgJsonApi source suggests a few possibilities:
This probably needs input from @fjgarlin.
Comment #7
fjgarlin commentedI think it was just to replicate what was done in here: https://git.drupalcode.org/project/drupalorg/-/blob/e31465608d1380345834...
ie: https://www.drupal.org/project/ckeditor
From the API endpoint is just a taxonomy term value, and then it’s really up to PB what to do with it. I think it can be cleaned up, and if we want to implement it, it can be done from scratch taking into account what is it we really want.
Currently, PB and filters only care about: active status and maintained status. In the early days, we were checking each possible value, and marking/showing some as "negative", but that changed with the introduction of other Plugins and it makes no sense anymore.
Should we want to show the descriptions of the values for the taxonomies, we could always do it, without the need to show a warning triangle.
Comment #8
phenaproximaThat makes sense to me. I think $warnings, as implemented in HEAD, is too generic and semantically meaningless. It really should be something like $flags, and that should be an enum case, or array of enum cases, that have specific meanings and can be rendered in specific ways by the Svelte code.
So +1 for removing it for now.
Comment #10
phenaproximaComment #11
phenaproximaComment #12
phenaproximaComment #13
phenaproximaComment #14
phenaproximaComment #15
chrisfromredfinThis looks really good from a code perspective, but at least one bug.
It seems like ones that should have the default/fallback logo are instead using the logo of the previous project that has one set. so in 2.0.x, Scheduler has a logo, but File Entity does not. Feeds does, but the three after Feeds do not, etc.
Once you get out of the top 100 you see a bunch that don't have logos (my favorite is going to page 5) on the drupalorg_jsonapi source.
Comment #16
phenaproximaComment #17
phenaproximaFIXED THAT. Super dumb bug -- the DrupalOrgJsonAPI source wasn't resetting
$logoas it loops over the projects. It does in HEAD; I just accidentally deleted a line in the refactoring.Since this was a bug introduced and fixed by this MR, I'm no longer entirely sure it benefits from a test. But I'll add one if you feel strongly. (Or we could do that in a follow-up if you're okay with that.)
Comment #18
tim.plunkettThis is a great clean-up. I'd like to see a test for #17, I'm actually surprised PHPStan didn't catch that. But that can happen in a follow-up.
Comment #20
chrisfromredfinWe made it!