Account created on 26 September 2024, 4 months ago
#

Merge Requests

More

Recent comments

🇮🇳India akulsaxena

The changes look good to me.
The 2 instances for hook_archiver_info() were removed and doc block has been updated accordingly.
The commenting looks fine now.
Moving to RTBC+.

🇮🇳India akulsaxena

Hi @paraderojether
Since Gitlab CI has been added to Drupal, It is used to find errors for coding standards. If the pipeline is passing, re-run the pipeline to test that it is passing and if the job succeeds, the standards are considered correct.
You can re-run the pipeline on this issue and if it passes, feel free to move it to RTBC+

🇮🇳India akulsaxena

Hello @danrod,

This issue has been dedicated to addressing coding standard fixes, and since the PHPCS pipeline is now passing, I have moved it to RTBC.

Regarding the CSPELL, ESLINT, PHPSTAN, and STYLEINT errors, wouldn't it be more efficient to create a separate issue to track the requested fixes, with the relevant changes outlined in the issue summary?

Thank you for your attention to this matter.

🇮🇳India akulsaxena

Hey @sandip-poddar
Thanks for reviewing the MR
I am a bit occupied right now, can someone work on this?
Thanks

🇮🇳India akulsaxena

Great

The deprecated $_TARGET_PHP has been replaced with $PHP_VERSION
unrequired .show-environment-variables has been removed
PHPCS, CSPELL and PHPSTAN are now running and not crashing at start

All pipelines are passing except cspell, for which an issue 📌 Make code pass cspell Active has already been created.

The status is already RTBC, i dont think we need to change that! This MR can be merged!
Thanks!

🇮🇳India akulsaxena

The changes LGTM
PHPCS, CSPELL and PHPSTAN are now running and not crashing at start
Moving to RTBC+

🇮🇳India akulsaxena

Thanks, 'operationtype' was added to cspell file and all the pipelines are green now
Changes LGTM
Moving to RTBC+

🇮🇳India akulsaxena

Great, thanks
The changes LGTM
The pipeline is now green as well
Moving to RTBC+

🇮🇳India akulsaxena

Tried reproducing the issue, was able to reproduce it
Applied the patch from the MR and it fixes the issue
All pipelines are also green
Moving it to RTBC+

🇮🇳India akulsaxena

The changes look good
But the PHPCS pipeline is failing due to whitespaces and empty lines in the added code in .install file
Can you please fix that

🇮🇳India akulsaxena

implicit nullables have been fixed for PHP 8.4
The changes LGTM
The composer pipeline is failing though but that seems unrealted to this issue
Moving to RTBC+

🇮🇳India akulsaxena

implicit nullables have been fixed for PHP 8.4
As stated, The PHPCS and PHPStan issues are unrelated to this issue.
The composer deprecation shows warning since The _TARGET_CORE variable is deprecated in GitLab Templates.
A new issue can be made for the same.
THis looks good to me
Moving to RTBC+

🇮🇳India akulsaxena

The rebase seems to hold for now
Can be moved to RTBC now, i worked on this so cant move it to RTBC, will wait for someone else to RTBC this.

🇮🇳India akulsaxena

Hey, the cspell pipeline is failing due to the word 'operationtype'
Can you please fix that?

🇮🇳India akulsaxena

The changes LGTM
But there seem to be some PHPCS error in the file. Seems like a single small error.
can you please fix it, moving it to NW for now

🇮🇳India akulsaxena

Hi @alok_singh
There seem to be some conflicts in the MR
Please resolve them

🇮🇳India akulsaxena

Hi @alok_singh
There seem to be some conflicts in the MR
Please resolve the conflicts

🇮🇳India akulsaxena

Hi @alok_singh
There seem to be some conflicts in the MR
Please resolve them

🇮🇳India akulsaxena

404s error solved
Moving to RTBC+

🇮🇳India akulsaxena

VERSION was removed from libraries.yml
Moving to RTBC+

🇮🇳India akulsaxena

The PHPCS pipeline is now passing and is now green
I guess this issue can now be merged as it would at least solve the breaking tests for a lot of merge requests in pending tickets.

🇮🇳India akulsaxena

Confirming that #4 work for Admin Toolbar 3.5.1 and Drupal 10.
#6 didnt work for me

🇮🇳India akulsaxena

Thanks @pfrenssen
The changes Look Good To Me.
The pipeline is all green now.
Since i worked on this issue, i wont be able to move it to RTBC
Will wait for someone else to review this

🇮🇳India akulsaxena

I resolved the conflicts but the pipelines were failing.
Fixed the phpstan pipeline, composer-lint and some phpcs errors.
There is still a warning in phpcs and phpunit is failing with the error

Unable to install modules: module 'webform_rest_test' is incompatible with this version of Drupal core.

Can someone please look into it.

🇮🇳India akulsaxena

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

🇮🇳India akulsaxena

Hi @acbramley
I reviewed your MR and all the implicit nullables deprecation errors have been fixed.
There seem to be some more pipeline errors which are being fixed in #3352346 📌 Fix the issues reported by phpcs Active .
This MR can be merged.
Moving to RTBC+

🇮🇳India akulsaxena

@astonvictor
Since the MR has been merged, can we move the issue to Fixed and close the issue?

🇮🇳India akulsaxena

Hey @niharika.s
I was already working on this issue and when i pushed my code, I didn't see anyone assigned but when I pushed the code and the page refreshed you were assigned to it.
I have already worked on it and provided the required MR
Moving it to NR and unassigning.

🇮🇳India akulsaxena

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

🇮🇳India akulsaxena

Hi @larowlan
I also worked on a child issue ( #3489287 📌 Method getMockForAbstractClass() is deprecated - replace in class EntityStorageBaseTest Active ) and was credited on that issue but I think you forgot to add credit here when you credited everyone who worked on a child issue.
Could you please look into it.
Thanks.

🇮🇳India akulsaxena

Changed the "process pipeline" back to "process definition" in the docs as per #15.
Please review

🇮🇳India akulsaxena

Hey @quietone,
I have added the return type to the new methods in the Stub file as requested.
Please review.

🇮🇳India akulsaxena

So should I change the "process pipeline" back to "process definition" in the docs?

🇮🇳India akulsaxena

Hi @alok_singh
I reviewed your MR and the issue seems to be fixed
Readmore and Add new comment were styled and spacing was added.
Moving it to RTBC+

🇮🇳India akulsaxena

I made the changes as suggested but the PHPCS pipeline is failing with the error:
Parameter $file has null default value, but is not marked as nullable.(SlevomatCodingStandard.TypeHints.NullableTypeForNullDefaultValue.NullabilityTypeMissing)
Thus, I think it needs to be made nullable according to GitLab CI.
Thus reverting it back to the original change and setting $file back to nullable.
The PHPCS pipeline is now green

🇮🇳India akulsaxena

The phpcs pipeline is passing and no errors/warnings were reported
Moving to RTBC+

🇮🇳India akulsaxena

Sorry, my commit message has wrong spelling for "Updated".
Anyways, Made the suggested changes
Please review.

🇮🇳India akulsaxena

Hi @clarkssquared
The Gitlab considers Dependency injections to be a part of PHPStan testing and not PHPCS
Since this issue is focused on coding standards and errors of codesniffer (PHPCS), the changes in the MR are good and solve them all.
Another issue can be created for PHPStan errors while this one can be moved to RTBC and then merged

🇮🇳India akulsaxena

Hi @yogesh.k
The MR has been merged and the issue solved, can we change the state of this issue to fixed and close the issue?

🇮🇳India akulsaxena

Made the changes suggested by @avpaderno
Had a doubt on one suggested change, left a comment reply on MR
Please review

🇮🇳India akulsaxena

Looks good to me.
I reviewed the MR and the ignore path was updated. The hardcoded path was changed in ignore path for SYMFONY_DEPRECATIONS_HELPER.
The PHPUnit test that were earlier failing have now been resolved and the PHPUnit pipeline is now green.
Moving it to RTBC+
Can be merged

🇮🇳India akulsaxena

Hi @yogesh.k
The MR has been merged and the issue solved, can we change the state of this issue to fixed and close the issue?

🇮🇳India akulsaxena

Verified MR!8, On mobile view content was provided space from both sides, fixed. RTBC++

🇮🇳India akulsaxena

Verified MR!9, On mobile view menu is not hidden from left side now, fixed. RTBC++

🇮🇳India akulsaxena

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

🇮🇳India akulsaxena

Hey,
I created an MR with the patch given above
PHPCS and PHPSTAN pipelines are failing
The composer dev dependencies have updated
Please take a look

🇮🇳India akulsaxena

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

🇮🇳India akulsaxena

@kul.pratap
Thanks for the Update.
The code was rebased from the 2.0.x branch to get the new pipeline to run.
All the pipelines are now passing and all phpcs issues have been resolved
Moving it to RTBC+

🇮🇳India akulsaxena

Hi
I reviewed the MR and the PHPCS errors have been fixed
The phpcs pipeline is now passing
Moving it to RTBC

🇮🇳India akulsaxena

Hi @alok_singh
I reviewed your MR and it seems to be working as expected
Footer was styled.
Pipeline is green
Moving to RTBC+

🇮🇳India akulsaxena

Hi @alok_singh
I reviewed your MR and it seems to be working as expected
Readmore and Add new comment were styled.
Pipeline is green
Moving to RTBC+

🇮🇳India akulsaxena

I reviewed your MR and the changes seem fine
Appropriate PHPdocs were added
Moving to RTBC+

🇮🇳India akulsaxena

Hi @dhruv.mittal

Thank you for updating the MR. I applied MR!6, it was applied smoothly and fixed all of the phpcs issues.
Thank you
Moving it to RTBC+

🇮🇳India akulsaxena

Updated the phpdocs
Please take a look now

🇮🇳India akulsaxena

Reviewed your changes in the MR
They look good to me
PHPUnit pipeline was failing, re-ran the pipeline and it passed
All pipelines are now green
Moving to RTBC+

🇮🇳India akulsaxena

Looks like the phpdocs werent updated
Added comment to the MR
Please update the phpdocs accordingly

🇮🇳India akulsaxena

The changes seem good to me
Moving to RTBC+

🇮🇳India akulsaxena

I reviewed your MR and the changes seem fine
Pipeline is all green
Moving to RTBC+

🇮🇳India akulsaxena

Nullable types were made exclusive
All pipelines are green
Moving to RTBC+

🇮🇳India akulsaxena

The nullable type was made exclusive
Moving to RTBC+

🇮🇳India akulsaxena

The change looks good to me
The correct status should be RTBC+
Moving it to RTBC

🇮🇳India akulsaxena

I modified the phpdocs as requested
Please review

🇮🇳India akulsaxena

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

🇮🇳India akulsaxena

@jvbrian
Will you be reviewing this? The licence file has been added and the required code for the same is already present in the composer.json file. Let me know if you find any other issue otherwise this can be set to RTBC and then merged.

🇮🇳India akulsaxena

Hey, I reviewed the MR
Changes look good
Nullable types were made explicit as suggested
Moving to RTBC+

🇮🇳India akulsaxena

I reviewed the MR
The changes look good
Explicit nullable types were used as suggested
Moving to RTBC+

🇮🇳India akulsaxena

@alyaj2a

Reviewed your MR
PHPCS errors given in Issue summary are now solved
Moving to RTBC+

🇮🇳India akulsaxena

Hi @alyaj2a

The changes in the MR seem fine
All 3 errors/warnings in the issue summary are now resolved
Moving it to RTBC+

🇮🇳India akulsaxena

Hi @alyaj2a

I reviewed your MR, no errors were found using the phpcs command in Issue summary.
Also, the phpcs pipeline is green.

Moving it to RTBC
Great work

🇮🇳India akulsaxena

Hi @alyaj2a
I tested your MR and it seems to work fine
PHPCS errors were resolved and pipeline is all green
Moving to RTBC

🇮🇳India akulsaxena

Hi @jvbrian
I already generated the MR and made the necessary changes.
The issue was already in Needs Review state.
All you have done is add two spaces extra in 2 different files, which is actually not required.
Please ensure if the issue you pick is already assigned to someone or is in needs review state and the work is complete, you should not work on it until it moves back to needs work state and is unassigned.

🇮🇳India akulsaxena

Hi
I have added the LICENSE.txt file as requested
The PHPCS and cspell pipelines are failing but an issue 📌 Fix cspell and phpcs issues Active has already been created for the same.
Please review and merge

🇮🇳India akulsaxena

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

🇮🇳India akulsaxena

The pipeline is now all green
All PHPCS errors have been solved
Please review

🇮🇳India akulsaxena

Hello
There are still some issues in the PHPCS pipeline
I'll work on them

Production build 0.71.5 2024