nexusnovaz → created an issue.
nexusnovaz → created an issue.
MR LGTM. Seems like a simple change and it shouldn't in theory affect anything else too much currently.
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
The change is simple and LGTM. Seems the error is unrelated through im not able to rerun the job?
I just realised the ticket i mentioned is under the same module. Closing this one as duplicate.
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.
nexusnovaz → created an issue.
This was a simple one to quickly do. Please review MR !16
I'll take a look at this!
Updated the MR to include the review comments
nexusnovaz → created an issue.
MR !14 is ready for review.
Thanks.
Marking as active now as it is merged
Postponing until 3511227 📌 Move Javascript out of PHP files Active is merged in to not cause conflicts.
nexusnovaz → created an issue.
The code in #3511227 lgtm
Code looks good to me and functions as expected
nexusnovaz → changed the visibility of the branch 3511227-move-javascript-out to hidden.
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
nexusnovaz → made their first commit to this issue’s fork.
Working on it
Hey, please review MR !9
nexusnovaz → created an issue.
nexusnovaz → created an issue.
nexusnovaz → created an issue.
nexusnovaz → created an issue.
I've added this and it appears to run. Is causing some issues which ill make as a new ticket. Please review MR !8
nexusnovaz → created an issue.
liampower → credited nexusnovaz → .
liampower → credited nexusnovaz → .
liampower → credited nexusnovaz → .
liampower → credited nexusnovaz → .
liampower → credited nexusnovaz → .
liampower → credited nexusnovaz → .
liampower → credited nexusnovaz → .
liampower → credited nexusnovaz → .
This is ready for review. Please see MR !7
liampower → credited nexusnovaz → .
nexusnovaz → made their first commit to this issue’s fork.
nexusnovaz → made their first commit to this issue’s fork.
nexusnovaz → made their first commit to this issue’s fork.
I believe MR !11339 is ready for review. I also found one more which I believe was also a mistake.
Ill take a quick look at this. Should be quick
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
Are we going to sort .routing by the route name, or the path?
nexusnovaz → made their first commit to this issue’s fork.
liampower → credited nexusnovaz → .
Running the grep command, there was only 20 functions. I've updated those and pushed my changed. Please can you review MR !10445
Ill take a look at this one
Hey, this is ready for review. Please see MR !10444
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!
Ill take a look at this
Updated the comment, but i believe tests will need an edit also
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
nexusnovaz → made their first commit to this issue’s fork.
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
nexusnovaz → made their first commit to this issue’s fork.
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.
nexusnovaz → changed the visibility of the branch 3032084-add-an-option to hidden.
nexusnovaz → made their first commit to this issue’s fork.
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
nexusnovaz → made their first commit to this issue’s fork.
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
Hey, can MR !10040 please be reviewed. Pipelines had a couple of fails, but after a retest they were fine.
Thanks!
I'll take a quick look at this
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.
nexusnovaz → made their first commit to this issue’s fork.
Hi @c.moisiadis, would you mind assigning credits for this issue.
Thanks.
Another issue with phpstan. has less information than #61
----------------------------------------------------------------------------------------------------
Running PHPStan on changed files.
PHPStan: failed
----------------------------------------------------------------------------------------------------
I am unsure how to fix this
An unrealted test was failing, but after a rerun, its all green now. Marking as needs review unless i've missed something else?
breidert → credited nexusnovaz → .
nexusnovaz → made their first commit to this issue’s fork.
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
nexusnovaz → made their first commit to this issue’s fork.
nexusnovaz → made their first commit to this issue’s fork.
I see now @smustgrave. Cheers and apologies!
Hopefully i understood #31 correctly. Added the small change! Please review
Converted patch #66 to an MR (!9533). Unsure where to start on tests, though i will be following
nexusnovaz → made their first commit to this issue’s fork.
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!
nexusnovaz → made their first commit to this issue’s fork.
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.
Updated deprecations to be 11.1.0. Could you please review MR !6742. Thanks!
nexusnovaz → made their first commit to this issue’s fork.
Hi, MR !9498 should now be ready for review.
Thanks
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