Problem/Motivation

If SDC get stable status it should integrate into the core namespace. This issue is to decide where the Exeptions should go.

Proposed resolution

I propose we move the Exeptions to Core\Theme\. So this would result in Core\Theme\ComponentNotFoundException

Remaining tasks

Create a merge request against this branch which is the meta issue for these changes.

Issue fork drupal-3365468

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

bbrala created an issue. See original summary.

dieterholvoet’s picture

dieterholvoet commented
Component: theme system » single directory components

plopesc made their first commit to this issue’s fork.

plopesc’s picture

plopesc
he/him
Primary language Spanish
commented
Status: Active » Needs review

MR available at https://git.drupalcode.org/issue/drupal-3352256/-/merge_requests/3

Namespace used for the exceptions is `Drupal\Core\Render\Component\Exception` instead of `Drupal\Them\Exception` in order to respect the already existing class aliases.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Don't think I can do it but can you open an MR against this issue? I see the branches and the MR link but doesn't appear they are connected? MR didn't have the tests run at least.

plopesc’s picture

plopesc
he/him
Primary language Spanish
commented
Status: Needs work » Needs review

Thank you for your review @smustgrave

This issue is a specific use case, where the MR is not created against the main drupal core origin.

Besides the MR mentioned above, against the META repo (https://git.drupalcode.org/issue/drupal-3352256/-/merge_requests/3), created a new one against this fork, which is not triggering Gitlab CI either(https://git.drupalcode.org/issue/drupal-3365468/-/merge_requests/1)

Already commented this with @e0ipso, who is aware of the situation.

I believe it is OK to leave this issue in NR status.

Thank you!

fjgarlin’s picture

fjgarlin commented

A few things that might help.
- The forks might need rebasing so the .gitlab-ci.yml files get into the codebase. I don't see them there so this might be the primary reason why they don't run.
- The second thing might be that the MRs need to be against the 11.x origin core branch, not fork to fork MRs. There is the condition of $CI_PROJECT_ROOT_NAMESPACE == "project" in the workflow file (https://git.drupalcode.org/project/gitlab_templates/-/blob/1.0.x/include...) which might have an impact.

smustgrave’s picture

smustgrave commented

Could #7 be done? Don't think this can be reviewed without knowing if it breaks existing test coverage.

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented

@smustgrave proper peer review will be done in the parent ticket. This MR is merging onto the feature branch of the parent. The reason to break things up is to have a place to have coherent discussions about the approach of each of the parts. Also so allow different people to work on different small parts.

Test coverage will be ensured in the step of merging the parent onto 11.x.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Purely looking at the code the deprecations seems correct. Without the tests can't tell if anything broke, but code wise looks fine.

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented
Status: Reviewed & tested by the community » Fixed

Merged! One more down.

Status: Fixed » Closed (fixed)

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