Tripal: Tripal Module Rating System

[spficklin]

There has been a discussion among the Project Management Committee (PMC) to implement a module rating system such that others who want to potentially use extension modules for Tripal can judge how well it might work for them before installing it. It's easy to say a module is Tripal v3 compatible but that doesn't mean it properly uses the Tripal API, or that it provides generic configuration such that another site could easily set it up and use it.

The PMC proposes a set of qualifications for rating modules as Bronze, Silver and Gold with different criteria that developers can use to judge their modules. The initial list of potential criteria are below. We open this up for discussion for all developers of Tripal extension modules to comment. Please comment if you think changes should be made to the lists below.

Bronze:

• Any Tripal version
  
  
  
  • Has a public release that follows Drupal release naming rules.
  
  
  • Should install on a Tripal site appropriate for the versions it supports.
  
  
  • Defines any custom tables or mviews in the install file (if applicable).
  
  
  • Provides Installation and admin instructions README.md (or RTD)
  
  
  • Has a license (distributed with module)
  
  
• Tripal v3 specific
  
  
  
  • Adds any needed controlled vocabulary terms in the install file (if applicable)
  
  

Silver:

• Any Tripal version
  
  
  
  • Follows Drupal code standards (how to measure this?)
  
  
  • Use tripal API functions (is this too generic)?
  
  
  • Uses the Chado API: Queries Chado using chado_generate_var, chado_expand_var, the ChadoRecord class or chado_xxx_record functions (except for complicated queries).
  
  
  • Provides an admin interface to allow for customization
  
  
• Tripal v3 specific:
  
  
  
  • Uses the TripalField class to add data to pages (if applicable).
  
  

Gold:

• Any Tripal version
  
  
  
  • Unit testing is implemented using PHPUnit with the TripalTestSuite or something similar.
  
  
  • Continuous integrating is setup (e.g. such as with TravisCI).
    
    
    
    
    
    • Required: execution of unit tests
    
    
    
    • Optional (but encouraged): code sniffing and testing coverage reports.
    
    
    
  
  
  • Project should have a public development branch (following Drupal naming rules) to encourage collaboration and pull requests.
  
  
  • Extensive documentation for the module (similar to Tripal User's Guide).
  
  
• Tripal v3 specific:
  
  
  
  • Imports data via Tripal's importer class
  
  
  • Tripal 3 fields are
    
    
    
    
    
    • Fully compatible with web services
    
    
    
    • The elementInfo function is fully implemented.
    
    
    
    • query and queryOrder functions fully implemented.
    
    
    
  
  
  • Web Services uses Tripal's Web Service Classes.
  
  

The standards will be given a version number. This way if the rules change we can refer to the version number for the rules.

Sample badges to post on a website?

edit: scratched the Chado API requirement from Silver

[bradfordcondon]

Follows Drupal code standards (how to measure this?)

this could be addressed by #714 . In the course of getting some sort of automated code coverage report, we would also get a style checker. We could then point users in the right direction for automated drupal style checking. Drupal has a codesniffer and maybe thats what we should be using, my first attempt at iwas pretty disastrous though 💥

Question, what is the benefit of using chado_generate_var, chado_expand_var, the ChadoRecord class? Is it that we cant assume the chado database is located at chado. ? I ask because I really really like using the Drupal PDO query builder. I this method less compatible than the tripal API? Could we talk about extending the drupal core database layer to make these available (or do we already?)

example usage:
https://github.com/NAL-i5K/tripal_eutils/blob/c1b4f40361e69470a10e8fd86acaced364df40a3/includes/repositories/EUtilsBioSampleRepository.php#L88-L92

[spficklin]

Question, what is the benefit of using chado_generate_var, chado_expand_var, the ChadoRecord class? Is it that we cant assume the chado database is located at chado.

Good point. Reflecting on this a bit I think maybe we should take out that requirement. Do we want to force folks to use the Chado API? They could just write straight SQL. What other benefit other than consistency is there? It's different from requiring the use of the Web Service class and TripalFields because those classes directly integrate with expected Tripal functionality.

We can have a chat about the Drupal PDO query builder perhaps on a different issue?

[mestato]

I'm a bit worried about "Should install on any Tripal site." What about modules that are specifically tied to a version? Lets say we develop a new module for v3. Since it won't install on v2, does it not qualify? Is it reasonable to expect developers to maintain modules for every supported Tripal version?

I guess I'm voting to change to something like "Should install on any Tripal site built from the target Tripal version specified." If keeping up with core is the essential goal, which is a good goal, maybe "Installs on the latest supported Tripal version" (although downgrading modules as new core versions emerge is another task i wouldn't want to deal with)

[bradfordcondon]

Should install on any Tripal site

we were writing these guidelines specifically thinking about Tripal 3.

Future versions of the docs will be tied to release so this will always be in reference to that specific version (in this case, tripal 3- these standards are only saying it will install on a Tripal 3 site, not Tripal 2 or future versions).

[spficklin]

Thanks @mestato for commenting! You make a good point. I think we can expand these rules to support all current version of Tripal.

[spficklin]

@mestato and @bradfordcondon I updated the rules at the top to be more Tripal version specific. Let me know if you are okay with this.

[bradfordcondon]

looks good! I'd think that what will happen is upon the next major Tripal release, we update the list to be about Tripal 4 compatibility and standards. The cool think about ReadTheDocs is the Tripal 3 list won't go anywhere: it will be listed at the same place on the 3.x version of ReadTheDocs.

edit- looking again, I think for the reason above we don't need general and v3 specific criteria. Unless you plan on hosting v2 awards on the same page, which i would discourage for the reason above. You could backport to v2, but the list of things to backport is growing with no one working on it.

What other benefit other than consistency is there?

@spficklin if theres no benefit then I don't see that we need to require their use. Many of those API functions use chado_generate_var which is thorough and therefore slower then necessary for a single value lookup for example. I'll note there are plenty of Chado API functions I do use, for example chado_insert_property.

[spficklin]

Good point @bradfordcondon so I think our badges should have the version number built in...

One benefit for using the Chado API is that we maintain backwards compatibility. Drupal did a major switch from 6 to 7 and we had to rewrite all of our SQL. But anyone who used the Chado API to pull out data did not. So there are benefits but it doesn't make your module any more compatible with Tripal core functionality.

[bradfordcondon]

further thoughts:

PHPUnit testing is implemented using the TripalTestSuite

I think if your tests are written using another framework (say Simpletest) thats still 👍

Continuous integrating is setup (e.g. such as with TravisCI)

lets be more specific. I now have guide for people to set up CI for:

• testing
• code sniffing
• testing coverage

i think that what we're most focused on is testing, though, so lets just say "CI testing" :)

Also, how about "handles uninstall gracefully" (something which i can be lazy about myself...)

[spficklin]

Okay, I changed the language above to read:

• Unit testing is implemented using PHPUnit with the TripalTestSuite or something similar.

I changed the language for CI to:

• Continuous integrating is setup (e.g. such as with TravisCI).
  
  
  
  • Required: execution of unit tests
  
  
  • Optional (but encouraged): code sniffing and testing coverage reports.
  
  

It's not that hard to encourage code sniffing, although it is a royal pain to fix if you have thousands. But I'm tempted to require an A or B rating for code sniffing. What say ye?

[bradfordcondon]

do you require sniffing is set up, or that its smell free? big difference. And do you speicfy the standard (Drupal) or do you allow PSR2...

[spficklin]

Oh good point. I think just setup would be nice. Yeah, I think it's important that folks know the Drupal coding standards as it helps encourage folks to think outside of their bubble.

[bradfordcondon]

public development branch (following Drupal naming rules)

I personally dislike Drupal naming rules for branches, and prefer to use the industry standard branch names (master instead of 7.x-3.x etc).

The releases we create follow Drupal rules however (or at least i try to, i still find them confusing).

Do you think this is an important requirement?

In general when Drupal does something different from industry in terms of project management i raise the same question. For example when we say release, i dont think we mean drupal release, but rather GitHub OR Drupal release, right?

At the same time, in terms of say code organization, syntax, or function naming, i do think its important to follow the Drupal standard specifically.

• Tripal v3 specific
  
  
  
  • Adds any needed controlled vocabulary terms in the install file (if applicable)
  
  

this is not a v3 specific requirement.

[spficklin]

I do think that following Drupal branch/release naming rules is important and would be a good requirement. This git repository is mirrored on Drupal.org and I don't see how we could have different releases on GitHub and Drupal. I think that would create confusion.... @laceysanderson your thoughts?

[bradfordcondon]

Drupal releases arent listed as part of the ratings, so if thats the main reason....

(we said releases, which i thought included releasing via github. If we did have a drupal release as a standard, i'd definitely think it would be gold.)

[almasaeed2010]

Hello all,

I have to agree with @bradfordcondon in that conforming to this naming convention is not necessary to produce quality modules. We recently published tripal elasticsearch on the Drupal website without having to convert to this way of naming. Drupal did not complain and it allowed us to pick the branch that we consider the main master branch.

I do agree though that versioning in the Drupal way drupal version - module version is an absolute must since that's what drush and the modules system require. I also agree that it's good practice to keep a branch for older versions of the module using the version name (e.g, 7.x-2.x for tripal 2 support).

Thanks!

[laceysanderson]

I believe the bronze standard should be releases which conform to the Drupal naming scheme: drupal version - module version but I think branch names should be flexible. I also think releases via GitHub should hold the same weight as releases via Drupal. Developers need to be able to choose the system (github/Drupal.org) they feel is best, as well as, the branch-ing scheme which works best for their development paradigm.

[bradfordcondon]

while we're talking about this, what version should the module start at? I notice that galaxy starts at 7.x-1.x (https://github.com/tripal/tripal_galaxy) whereas, say, jbrowse started at 7.x-2.x and is now at 7.x-3.x . Functional annotation modules also follow the tripal version (but they've been around and their releases correspond to Tripal releases).

Put another way, should the module version mirror the tripal version its intended to work with, or follow the "Correct" Drupal versioning? I can see arguing for either side, although im in favor of mirroring the Tripal version because users don't care about the module in a vacuum! They care about which Tripal version it works with! And major releases are likely to correspond with Tripal releases anyway.

we were trying to version tripal_HQ's and couldnt decide between starting at 7.x-1.x and 7.x-3.x !

[laceysanderson]

I think we can make a recommendation for where versions should start but not a requirement :-) I'm very inconsistent here as my views keep changing 😝 not sure what I would even recommend

[almasaeed2010]

My vote would be to start from 1.x and inform the user that this version works with tripal X. In my mind at least I wouldn't release a new module starting from v3 when v1 and 2 don't exist 😅

[guignonv]

Follows Drupal code standards (how to measure this?)

this could be addressed by #714 . In the course of getting some sort of automated code coverage report, we would also get a style checker. We could then point users in the right direction for automated drupal style checking. Drupal has a codesniffer and maybe thats what we should be using, my first attempt at iwas pretty disastrous though 💥

That's what I use (Drupal code sniffer) for my modules and it's just a pain the first time(s). :-) Then you learn and know what to do or not to do and the next passes are usually easy to fix. And then, your code is easy to read by everybody and that's the very important point. Regardless you like it or not (I personally don't like all the Drupal coding standards), it's the common way to share your code, just like English (and not French! ;-) ) is used for the documentation. That's how it should be presented to people writing extensions.

Yet another link on how to install/use it: https://www.drupal.org/project/coder

[laceysanderson]

Github Style badges:

[laceysanderson]

This is now live! See it here https://tripal.readthedocs.io/en/latest/extensions/module_rating.html

And the badges now work 🎉

[spficklin]

Wonderful! I'm going to mark this as closed. If we need to adjust the text for the ratings I think it would be best to start a new issue. But if anyone disagrees please reopen.

[bradfordcondon]

what would you all think about including this markdown snippet for easy issue tracking

## Bronze

- [ ] Has a public release.
- [ ] Should install on a Tripal site appropriate for the versions it supports.
- [ ] Defines any custom tables or materialized views in the install file (if applicable).
- [ ] Adds any needed controlled vocabulary terms in the install file (Tripal3).
- [ ] Provides Installation and admin instructions README.md (or RTD).
- [ ] Has a license (distributed with module).

## Silver
- [ ] Follows basic Drupal Coding standards; specifically, code format and API documentation.
- [ ] Uses Tripal API functions. Specifically, it should use the Chado Query API for querying chado (if using chado as the storage system)
- [ ] Tripal Jobs API for long running processes
- [ ] TripalField class to add data to pages (Tripal3). (Tutorial)
- [ ] Provides ways to customize the module (e.g. drush options, field/formatter settings, admin UI).
- [ ] Latest releases should follow Drupal naming best practices. e.g. first release for Drupal 7 should be: 7.x-1.x.

## Gold
- [ ] Extensive documentation for the module (similar to Tripal User’s Guide). ( Tutorial)
- [ ] Unit testing is implemented using PHPUnit with the TripalTestSuite or something similar.
- [ ] Continuous integration is setup (e.g. such as with TravisCI).
- [ ] Imports data via Tripal’s importer class (Tripal3) (Tutorial).
- [ ] Tripal 3 fields are (Tutorial) Fully compatible with web services.
- [ ] The elementInfo function is fully implemented.
- [ ] The query and queryOrder functions fully implemented.
- [ ] Web Services uses Tripal’s Web Service Classes (Tripal3). (Tutorial)
- [ ] Code sniffing and testing coverage reports (optional but encouraged).
- [ ] Drupal.org vetted release (optional but encouraged).