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+.
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+
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.
Hey @sandip-poddar
Thanks for reviewing the MR
I am a bit occupied right now, can someone work on this?
Thanks
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!
Cool, Since it isn't required, it can be removed! go ahead!
The changes LGTM
PHPCS, CSPELL and PHPSTAN are now running and not crashing at start
Moving to RTBC+
Thanks, 'operationtype' was added to cspell file and all the pipelines are green now
Changes LGTM
Moving to RTBC+
Great, thanks
The changes LGTM
The pipeline is now green as well
Moving to RTBC+
Great, thanks!
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+
Tested and looks good to me
Moving it to RTBC+
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
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+
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+
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.
Hey, the cspell pipeline is failing due to the word 'operationtype'
Can you please fix that?
Looks good now
Setting to RTBC+
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
Hi @alok_singh
There seem to be some conflicts in the MR
Please resolve them
Hi @alok_singh
There seem to be some conflicts in the MR
Please resolve the conflicts
Hi @alok_singh
There seem to be some conflicts in the MR
Please resolve them
Removed out of scope changes
404s error solved
Moving to RTBC+
VERSION was removed from libraries.yml
Moving to RTBC+
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.
Confirming that #4 work for Admin Toolbar 3.5.1 and Drupal 10.
#6 didnt work for me
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
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.
akulsaxena → made their first commit to this issue’s fork.
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+
@astonvictor
Since the MR has been merged, can we move the issue to Fixed and close the issue?
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.
akulsaxena → made their first commit to this issue’s fork.
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.
The PHPStan is now passing, Moving it to NR
Removed the Baseline entry for StubEntityStorageBase as suggested
Changed the "process pipeline" back to "process definition" in the docs as per #15.
Please review
Hey @quietone,
I have added the return type to the new methods in the Stub file as requested.
Please review.
akulsaxena → made their first commit to this issue’s fork.
So should I change the "process pipeline" back to "process definition" in the docs?
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+
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
The phpcs pipeline is passing and no errors/warnings were reported
Moving to RTBC+
Added gitlab-ci file and rebased
Sorry, my commit message has wrong spelling for "Updated".
Anyways, Made the suggested changes
Please review.
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
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?
Made the changes suggested by @avpaderno
Had a doubt on one suggested change, left a comment reply on MR
Please review
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
Reviewing this
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?
Verified MR!8, On mobile view content was provided space from both sides, fixed. RTBC++
Verified MR!9, On mobile view menu is not hidden from left side now, fixed. RTBC++
Moving it to NR
Sorry, missed the change from '-' to '_'
Made necessary changes
Please review
akulsaxena → made their first commit to this issue’s fork.
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
akulsaxena → made their first commit to this issue’s fork.
@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+
Hi
I reviewed the MR and the PHPCS errors have been fixed
The phpcs pipeline is now passing
Moving it to RTBC
Hi @alok_singh
I reviewed your MR and it seems to be working as expected
Footer was styled.
Pipeline is green
Moving to RTBC+
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+
I reviewed your MR and the changes seem fine
Appropriate PHPdocs were added
Moving to RTBC+
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+
Updated the phpdocs
Please take a look now
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+
Looks like the phpdocs werent updated
Added comment to the MR
Please update the phpdocs accordingly
Added comments to MR, please look into it
The changes seem good to me
Moving to RTBC+
I reviewed your MR and the changes seem fine
Pipeline is all green
Moving to RTBC+
Nullable types were made exclusive
All pipelines are green
Moving to RTBC+
The nullable type was made exclusive
Moving to RTBC+
The change looks good to me
The correct status should be RTBC+
Moving it to RTBC
I modified the phpdocs as requested
Please review
akulsaxena → made their first commit to this issue’s fork.
@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.
Hey, I reviewed the MR
Changes look good
Nullable types were made explicit as suggested
Moving to RTBC+
I reviewed the MR
The changes look good
Explicit nullable types were used as suggested
Moving to RTBC+
@alyaj2a
Reviewed your MR
PHPCS errors given in Issue summary are now solved
Moving to RTBC+
Hi @alyaj2a
The changes in the MR seem fine
All 3 errors/warnings in the issue summary are now resolved
Moving it to RTBC+
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
Hi @alyaj2a
I tested your MR and it seems to work fine
PHPCS errors were resolved and pipeline is all green
Moving to RTBC
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.
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
akulsaxena → made their first commit to this issue’s fork.
The pipeline is now all green
All PHPCS errors have been solved
Please review
Hello
There are still some issues in the PHPCS pipeline
I'll work on them