Skip to content

Merging pull requests

wvengen edited this page Nov 12, 2013 · 6 revisions

Maintainers of the foodcoops/foodsoft repository are responsible for handling contributions. When a pull request is submitted, a maintainer reviews the changes (see Developing Guidelines).

Because we use localeapp to manage translations, maintainers need to take some care when merging - so that translations end up in localeapp, and they don't overwrite work that translators may have done. The following steps can help here - let's consider maintainer Molly handling a pull request by developer Devon.

  1. Molly reviews the pull request. If she, for example, finds plain-text that needs to be i18n-ised, she comments on the pull request and asks Devon to add some changes.
  2. When the pull is ready for inclusion into foodcoops/foodsoft, she makes sure that there is no difference between localeapp and the source tree localeapp = source tree
  3. She checks out master from foodcoops/foodsoft
  4. She does a localeapp pull (or perhaps localeapp update) to get the latest localeapp changes
  5. If there is any difference (git diff shows anything), she reviews the changes and commits them.
  6. Now localeapp and the source tree have the same idea about texts.
  7. Molly merges the pull request into master.
  8. Molly updates localeapp
  9. she pushes changed locale files (at the very least the English one) to localeapp using localeapp push
  10. important: she reviews the pull request for deleted i18n keys, and deletes those in the localeapp web interface (this does not happen automatically!)
  11. Molly makes makes sure pulling translations from localeapp result in same files as in source tree localeapp = source tree
  12. Molly retrieves the locale files from localeapp using localeapp pull
  13. If there is any difference (git diff shows anything), she checks that those are syntactic changes and commits them with commit message "fixup localeap roundtrip".

This makes sure that translation changes from localeapp are clearly distinguishable from merges. See also this mailing list thread. Perhaps we can someday have a rake task or scripts to do this automatically.

Plugins

When merging a pull request for a plugin, there are some other things to check and do:

  • It needs its own LICENSE file, unless it is equal to foodsoft's (GPL3).
  • A plugin's locales go into its own config/locales/en.yml (see here). After merging:
    1. Import this file into localeapp
    2. Pull the localeapp changes into foodsoft and commit them. This will copy the strings into foodsoft's texts, as well as any translations
  • Check that it is not enabled/included in the Gemfile, unless it's a default plugin (like the wiki).
  • There is no testing of plugins, yet. When in doubt, checkout and test locally with the plugin enabled.
Clone this wiki locally