Problem/Motivation

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#25 phpcsfix.patch12.64 KBanjali mehta
#6 interdiff_2-6.txt14.55 KBannmarysruthy
#6 avoid-t-calls-in-class-3344896-6.patch21.07 KBannmarysruthy
#2 3344896-2.patch6.25 KBgayatri chahar

Issue fork xnumber-3344896

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

gayatri chahar created an issue. See original summary.

gayatri chahar’s picture

gayatri chahar commented
Assigned: gayatri chahar » Unassigned
Status: Active » Needs review
StatusFileSize
new 3344896-2.patch6.25 KB

Patch created. Please review it

hardikpandya’s picture

hardikpandya commented
Status: Needs review » Needs work

There are still procedural t() calls in src/Plugin/Field/FieldWidget/XnumberWidget.php and src/Tests/XnumberFieldTest.php. Please fix them!

annmarysruthy’s picture

annmarysruthy commented
Assigned: Unassigned » annmarysruthy
annmarysruthy’s picture

annmarysruthy commented
annmarysruthy’s picture

annmarysruthy commented
StatusFileSize
new avoid-t-calls-in-class-3344896-6.patch21.07 KB
new interdiff_2-6.txt14.55 KB

created a new patch for changing all the t() calls. Please review

annmarysruthy’s picture

annmarysruthy commented
Assigned: annmarysruthy » Unassigned
Status: Needs work » Needs review
drugan’s picture

drugan commented
Status: Needs review » Needs work

Thanks for your contribution!

Please ignore t() in the src/Tests/XnumberFieldTest.php file. Instead you can create new Drupal 10 compatible test files here:

#3299531: Automated Drupal 10 compatibility fixes

Also, please follow fork -> merge request workflow which is the new way of making changes in the code:

https://www.drupal.org/docs/develop/git/using-git-to-contribute-to-drupa...

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

annmarysruthy’s picture

annmarysruthy commented
Assigned: Unassigned » annmarysruthy

annmarysruthy opened merge request !4

annmarysruthy’s picture

annmarysruthy commented
Assigned: annmarysruthy » Unassigned
Status: Needs work » Needs review

MR !4 raised to avoid t() calls inside class. please review

urvashi_vora’s picture

urvashi_vora commented
Assigned: Unassigned » urvashi_vora
Status: Needs review » Needs work

Hi @annmarysruthy,

I reviewed MR 4.

Steps performed while reviewing:-
1. Taken clone of issue branch
2. Reviewed the changes as per Merge Request 4.
3. I still got the "t() calls should be avoided in classes" error for /src/Tests/XnumberFieldTest.php.
4. Attaching the screenshot for reference.

Only local images are allowed.

Test Result:- Needs Work.
Moving it to Needs Work.

I am working on it, assigning it to myself.

Thanks for the work.

urvashi_vora opened merge request !5

urvashi_vora’s picture

urvashi_vora commented
Assigned: urvashi_vora » Unassigned
Status: Needs work » Needs review

PLease review the MR.

drugan’s picture

drugan commented
Status: Needs review » Needs work

Hi,

After the 2.0.0-beta1 release 8.x version of the module will not get changes anymore except critical ones.

Please, fix merge issues and do not forget to follow Drupal git commit guidelines.

Hint: Just copy it from the prompt below.

drugan closed merge request !4

bharath-kondeti made their first commit to this issue’s fork.

bharath-kondeti’s picture

bharath-kondeti commented
Status: Needs work » Needs review
drugan’s picture

drugan commented
Status: Needs review » Needs work

As you see it is still unmergeable.

Please, properly merge upstream first.

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

elber’s picture

elber
Primary language Portuguese, Brazil
Location Brazil
commented
Status: Needs work » Needs review
drugan’s picture

drugan commented
Status: Needs review » Needs work

Please, fix Drupal coding standards errors introduced by your changes.

Anjali Mehta made their first commit to this issue’s fork.

anjali mehta’s picture

anjali mehta commented
Status: Needs work » Needs review
StatusFileSize
new phpcsfix.patch12.64 KB

Added patch to fix warnings and errors reported by phpcode sniffer

nitin_lama’s picture

nitin_lama
he/him
Location India
commented
Assigned: Unassigned » nitin_lama
Status: Needs review » Needs work
nitin_lama’s picture

nitin_lama
he/him
Location India
commented
Status: Needs work » Needs review
nitin_lama’s picture

nitin_lama
he/him
Location India
commented
Assigned: nitin_lama » Unassigned
drugan’s picture

drugan commented
Status: Needs review » Needs work

Changes in the README.md file are not related to this issue which says:

t() calls should be avoided in class

nitin_lama’s picture

nitin_lama
he/him
Location India
commented
Assigned: Unassigned » nitin_lama
nitin_lama’s picture

nitin_lama
he/him
Location India
commented
Assigned: nitin_lama » Unassigned
Status: Needs work » Needs review
elber’s picture

elber
Primary language Portuguese, Brazil
Location Brazil
commented

Please revise

drugan’s picture

drugan commented
Status: Needs review » Needs work

The module has good test coverage, please run tests before asking for review.

elber’s picture

elber
Primary language Portuguese, Brazil
Location Brazil
commented
Status: Needs work » Needs review

Please revise.

drugan’s picture

drugan commented
Status: Needs review » Needs work

Please address the comment above.

elber’s picture

elber
Primary language Portuguese, Brazil
Location Brazil
commented
Status: Needs work » Needs review

please revise

drugan’s picture

drugan commented
Status: Needs review » Needs work
elber’s picture

elber
Primary language Portuguese, Brazil
Location Brazil
commented
Status: Needs work » Needs review

Please revise again

elber’s picture

elber
Primary language Portuguese, Brazil
Location Brazil
commented

rebase completed

  • drugan committed d9a8860d on 2.0.x authored by urvashi_vora 
    'Issue #3344896 by elber, gayatri chahar, drugan: t() calls should be...
drugan’s picture

drugan commented
Status: Needs review » Fixed
drugan’s picture

drugan commented

Thanks everyone!

Status: Fixed » Closed (fixed)

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