🇬🇧United Kingdom @nexusnovaz

Account created on 2 October 2023, over 1 year ago
  • Back End Developer at Zoocha 
#

Merge Requests

More

Recent comments

🇬🇧United Kingdom nexusnovaz

MR LGTM. Seems like a simple change and it shouldn't in theory affect anything else too much currently.

🇬🇧United Kingdom nexusnovaz

Hey @charlliequadros, i've made a slight modification as the code is actually JS and not PHP, but it appears to still work. Care to review/test again

🇬🇧United Kingdom nexusnovaz

The change is simple and LGTM. Seems the error is unrelated through im not able to rerun the job?

🇬🇧United Kingdom nexusnovaz

I just realised the ticket i mentioned is under the same module. Closing this one as duplicate.

🇬🇧United Kingdom nexusnovaz

This is a ticket created as a kind of duplicate for https://www.drupal.org/project/documentation/issues/3489514 📌 Suggestion for: (2827301) 3.4. Preparing to Install Active as the documentation page says to open a new ticket on this module and upload the file here.

🇬🇧United Kingdom nexusnovaz

This was a simple one to quickly do. Please review MR !16

🇬🇧United Kingdom nexusnovaz

I'll take a look at this!

🇬🇧United Kingdom nexusnovaz

Updated the MR to include the review comments

🇬🇧United Kingdom nexusnovaz

MR !14 is ready for review.

Thanks.

🇬🇧United Kingdom nexusnovaz

Marking as active now as it is merged

🇬🇧United Kingdom nexusnovaz

Postponing until 3511227 📌 Move Javascript out of PHP files Active is merged in to not cause conflicts.

🇬🇧United Kingdom nexusnovaz

The code in #3511227 lgtm

🇬🇧United Kingdom nexusnovaz

Code looks good to me and functions as expected

🇬🇧United Kingdom nexusnovaz

nexusnovaz changed the visibility of the branch 3511227-move-javascript-out to hidden.

🇬🇧United Kingdom nexusnovaz

Hey, please review MR !13. I'm unsure if i need to add more information to the readme or the help page, but this is a good start i believe

🇬🇧United Kingdom nexusnovaz

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

🇬🇧United Kingdom nexusnovaz

Hey, please review MR !9

🇬🇧United Kingdom nexusnovaz

I've added this and it appears to run. Is causing some issues which ill make as a new ticket. Please review MR !8

🇬🇧United Kingdom nexusnovaz

This is ready for review. Please see MR !7

🇬🇧United Kingdom nexusnovaz

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

🇬🇧United Kingdom nexusnovaz

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

🇬🇧United Kingdom nexusnovaz

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

🇬🇧United Kingdom nexusnovaz

I believe MR !11339 is ready for review. I also found one more which I believe was also a mistake.

🇬🇧United Kingdom nexusnovaz

Ill take a quick look at this. Should be quick

🇬🇧United Kingdom nexusnovaz

I'm having the same issue on this. It appears to only be when an image is added with the centre style. If the image has a caption, the styling works fine. So im wondering if its the figure/article styling.
This code below works:

<figure role="group" class="caption caption-drupal-media align-center">
  <article class="media media--type-image media--view-mode-full-width">
    <div class="field field--name-field-media-image field--type-image field--label-visually_hidden">
      <div class="field__label visually-hidden">Image</div>
      <div class="field__item">
        <img loading="lazy" src="<A URL>/857-630x200.jpg?itok=Fb7MC4Ym" width="630" height="200" alt="test" class="image-style-full-width">
      </div>
    </div>
  </article>
  <figcaption>Caption option</figcaption>
</figure>

This code below does not work:

<article class="align-center media media--type-image media--view-mode-full-width">      
  <div class="field field--name-field-media-image field--type-image field--label-visually_hidden">
    <div class="field__label visually-hidden">Image</div>
    <div class="field__item">
      <img loading="lazy" src="<A URL>/857-630x200.jpg?itok=Fb7MC4Ym" width="630" height="200" alt="test" class="image-style-full-width">
    </div>
  </div>
</article>

This is using the Gin 8.x-3.0-rc16 & ckeditor5 11.1.1

🇬🇧United Kingdom nexusnovaz

Are we going to sort .routing by the route name, or the path?

🇬🇧United Kingdom nexusnovaz

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

🇬🇧United Kingdom nexusnovaz

Running the grep command, there was only 20 functions. I've updated those and pushed my changed. Please can you review MR !10445

🇬🇧United Kingdom nexusnovaz

Had a little issue with PhpStorm auto formatting, but thats been resolved and the MR should only contain changes made to this now. Please could it be reviewed.

Thanks!

🇬🇧United Kingdom nexusnovaz

Updated the comment, but i believe tests will need an edit also

🇬🇧United Kingdom nexusnovaz

I was unsure what was meant by fatal error and couldn't find anything specific for loading exceptions so have gone with a normal \Exception. Please could MR !10260 be reviewed.

Thanks

🇬🇧United Kingdom nexusnovaz

Hey, I've made a start on this.

Hopefully im reading this right and understanding the situation. I would assume this just means that it generates the config as
module_name.entity_id if no config_prefix is passed in. Ill set to review in case its right. Please see MR !10258

🇬🇧United Kingdom nexusnovaz

I was trying to write some tests unfortunately I can't seem to get PhpStorm to work with it. I've rolled #12 into the MR !39. I'm going to keep trying to see if i can get mine to work, but if anyone gets it first, feel free.

🇬🇧United Kingdom nexusnovaz

nexusnovaz changed the visibility of the branch 3032084-add-an-option to hidden.

🇬🇧United Kingdom nexusnovaz

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

🇬🇧United Kingdom nexusnovaz

Hi,

I believe I got quite a few of them, though its actually rather difficult to tell. For example: "The giftcard storage schema." should this be changed or kept the same? Same with "Access control handler for giftcard transactions.". I'm going to mark as needs review for the words, im unsure why the tests are failing. Something about views page 2 schema. If this isn't the correct workflow please let me know and ill make a note for future.

I also noticed another issue which seems out of scope for this ticket but im unsure how to go about creating a bug/ticket for it. In src/Entity/GitftcardType:76 there is `The the` in a comment.

Thanks

🇬🇧United Kingdom nexusnovaz

Hey,

Thanks for that. I wasn't too sure about the return type but also wasn't sure what the correct answer was. Thanks for the link to the coding standards for this issue too! Setting back to needs review before pipeline has run, but it should be good!

Thanks

🇬🇧United Kingdom nexusnovaz

Hey, can MR !10040 please be reviewed. Pipelines had a couple of fails, but after a retest they were fine.

Thanks!

🇬🇧United Kingdom nexusnovaz

Hey @joachim,

Apologies, was waiting for the pipeline to pass/fail before changing and requesting code review on this! Could i confirm the workflow process? Is it When i start working on an issue, i assign to myself and leave as active? Or change to needs work?

Also, i believe MR !10007 is ready for review. There was a failed test but that seems unrelated to this issue, so im hoping a rerun of that test will fix it.

Thanks.

🇬🇧United Kingdom nexusnovaz

Hi @c.moisiadis, would you mind assigning credits for this issue.

Thanks.

🇬🇧United Kingdom nexusnovaz

Another issue with phpstan. has less information than #61

----------------------------------------------------------------------------------------------------

Running PHPStan on changed files.

PHPStan: failed

----------------------------------------------------------------------------------------------------

I am unsure how to fix this

🇬🇧United Kingdom nexusnovaz

An unrealted test was failing, but after a rerun, its all green now. Marking as needs review unless i've missed something else?

🇬🇧United Kingdom nexusnovaz

Hey, updated publishcontent.settings.yml to make ui_checkboxes true. Should the change to the .install be reverted back to 0?

Will mark as needs review in case this is all good. Thanks

🇬🇧United Kingdom nexusnovaz

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

🇬🇧United Kingdom nexusnovaz

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

🇬🇧United Kingdom nexusnovaz

Hopefully i understood #31 correctly. Added the small change! Please review

🇬🇧United Kingdom nexusnovaz

Converted patch #66 to an MR (!9533). Unsure where to start on tests, though i will be following

🇬🇧United Kingdom nexusnovaz

Hey, i fixed the test and had to reformat the exception message due to it printing ()->getLabel() rather than the actual label. It took some tests a couple of retries due to failures, but they passed in the end. Please could the MR be reviewed. Thanks!

🇬🇧United Kingdom nexusnovaz

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

🇬🇧United Kingdom nexusnovaz

Hey, made the CR which can be found here: https://www.drupal.org/node/3475054 . Also replaced the issue links in the MR with this link. Please can this be reviewed again.

🇬🇧United Kingdom nexusnovaz

Updated deprecations to be 11.1.0. Could you please review MR !6742. Thanks!

🇬🇧United Kingdom nexusnovaz

Hey @joachim,

How should i mark the default? Would it be (default) before the option? Does the default need to be the first in the list? (default) at the end of the item? I couldn't find anything in the coding standards, unsure if maybe i've missed it.

Thanks

Production build 0.71.5 2024