Much more flexible consent forms
#4 Updated by Ward Vandewege over 6 years ago
- Assigned To set to Phil Hodgson
Since this code is not in a separate branch (I think?), I reviewed these commits:
- commit 435659ab7864a40447b728e6e802de1687250ff6 introduces whitespace at the end of a few lines.
- commit 435659ab7864a40447b728e6e802de1687250ff6:
I like where this is going. A few thoughts:
a) the documentation in lib/site_specific/validations.rb is great. I wonder if it belongs on the Tapestry wiki in a 'Customization' section that should also include the bits about internationalization and theming (already present on the wiki, feel free to reorganize). The validations.rb file could have a pointer to the docs. What do you think? I'm not 100% sure on the best place for these docs: it makes sense to have them close to the code, but customizing Tapestry is going to be a very common occurrence, and I think it would be valuable to have all relevant documentation in one place. On the wiki for now, maybe in the future we'll have a proper documentation tree like we have for Arvados.
b) Is there a way to have people put custom validations in a separate file? That would be preferrable to avoid git merge conflicts and facility upgrades.
c) the documentation in validations.rb does not yet refer to the file overlay scheme from #2518. It probably should?
- commit b05f26f66885d5a5ba4e5dfd2e68207df9bd7fe4 introduces whitespace at the end of a few lines.
#6 Updated by Ward Vandewege about 6 years ago
Reviewing the 2501-flexibler-consent branch:
in 7dd2baccae83c8b23de04811019d125c65f36b80 you mention
ActiveSupport::Dependencies.explicitly_unloadable_constants << 'SiteSpecific::Validations'
which people should add to their development.rb. Can you add it to config/development.rb.example? It seems that line was dropped from the doc on https://arvados.org/projects/tapestry/wiki/Customization -- was that intentional?
Other than that, this looks great and I think it's ready to merge. Thanks!