search

tripal / tripal_doc

Official Documentation for the Tripal Platform
https://tripaldoc.readthedocs.io/en/latest/
GNU General Public License v3.0
2 stars 3 forks source link

Make naming conventions for fields clearer #53

Closed Ferrisx4 closed 6 months ago

Ferrisx4 commented 7 months ago

Delineate more carefully between fields from core or extension modules

Should satisfy the comments by @laceysanderson and @spficklin here: https://github.com/tripal/tripal_doc/issues/27 and here https://github.com/tripal/tripal_doc/pull/28

dsenalik commented 7 months ago

For recent fields and all the ones I have created I do not have "Type" in the file name. ChadoOrganismDefault.php instead of ChadoOrganismTypeDefault.php Should I change that?

laceysanderson commented 7 months ago

We should be consistent so that implies yes 😘 The Type is the name to make it clear this is the fieldtype class not the widget or formatter.

That said, the name does sound better without the "Type" and it is clearly the field type as it doesn't say widget or formatter. Thoughts @dsenalik, @tripal/project-managent-commitee-pmc? Do we change it to be more concise or keep it the same to be more clear?

dsenalik commented 7 months ago

I vote for consistency over conciseness.

laceysanderson commented 7 months ago

We will have consitency regardless. 🤪 Its just do we change your new fields or do we change the existing fields to match the changed standard? 🤔

dsenalik commented 7 months ago

Okay, for consistency I think ChadoTextTypeItem be ChadoTextTypeDefault

and ChadoTextTypeWidget should be ChadoTextWidgetDefault

and so on and so forth for all four of ChadoBooleanTypeItem ChadoIntegerTypeItem ChadoStringTypeItem ChadoTextTypeItem I can do a PR to just change all existing fields to the "old" standard (add Type) or the "new" standard (like most are now except these four) as the PMC advises.

Ferrisx4 commented 7 months ago

If it's not too much of a task to make the change, and it won't break other things and be a headache for a while, I think we should have 'Type' in the name.

It may not be clear to new developers that this is the Type file simply because it's not one of the other two (we are assuming everybody knows what all three types are).

dsenalik commented 7 months ago

Looking a little bit later in the docs, we would also want to change the internal naming to match 20231211_fieldnaming this would become my_field_type_default and we would want a corresponding pair of tables for core and extension here also I will do a PR to implement this in Tripal if we agree.

dsenalik commented 6 months ago

Tripal PR https://github.com/tripal/tripal/pull/1709 which was just merged standardizes all names so these documentation changes are correct. I can add the changes I suggested in https://github.com/tripal/tripal_doc/pull/53#issuecomment-1850522295

Ferrisx4 commented 6 months ago

and thank you @dsenalik!