Allow passing of properties to the phraseapp API that are allowed

[gordoncl]

For whatever reason, phraseapp turns on zero_plural_form_enabled by default. This sometimes causes editors to add strings for the zero case even though we don't ever read them for that language. I'd like to turn it off.

This PR adds the ability to setup a phraseapp project and pass an audited subset of options that phraseapp allows to the create project API. There were some I didn't include, like main_format, which we already define as file-format. Or, things like enable-branching, which seem like they would interfere with the purposes of the application.

Presuming this branch gets approved in it's currently incarnation, I would need to update the frontend to include --zero_plural_form_enabled=false as an argument. I'd be interested to know if you think I should just make false the default value here. Because if I did, I could call it a day. It's dubious whether we want to allow all these options or just gloss over it and cater to our case 🤷

API docs

https://developers.phrase.com/api/#post-/projects

[gordoncl]

I tried to apply YARD style syntax here but I'm not sure if we have a standard for documenting our ruby code.

[gordoncl]

This is probably unnecessary now that I'm looking at it, **opts shouldn't in theory contain name or main_format. But it still might be nice to have. Thoughts?

[gordoncl]

I had a couple of ideas for this originally.

The first idea was just to allow the use of -- and anything beyond it would be passed directly to phraseapp.

phraseapp_updater setup \
  --phraseapp_project_name=... \
  --parent_commit=... \
  --phraseapp_api_key=... \
  --file-format=json \
  path/to/locales
  -- --zero_plural_form_enabled=false

But, I wasn't sure if thor would interpret the false here or if I would be able to convert those options to anything without significant magic sauce. So I went ahead and added my own definitions.

To try to make people's lives slightly less miserable, I added all the optional options that didn't pertain to branching.

My second idea was only to add the options I actually wanted to change. 🤷

[chrisandreae]

only to add the options I actually wanted to change.

That'd be a pretty reasonable choice, to be honest. We use so little of Phraseapp that exposing too many of its options feels like it's more likely to cause rather than save trouble.

[gordoncl]

I added a commit which does this. method_options actually returns a meta object with the information you provided. So I just capture them into a collection inline and use that later:

PHRASEAPP_CREATE_PROJECT_OPTIONS = [
  method_option(:zero_plural_form_enabled, type: :boolean, desc: 'Displays the input fields for the \'ZERO\' plural form for every key as well although only some languages require the \'ZERO\' explicitly.')
]

def setup
  ...
  phraseapp_create_options = PHRASEAPP_CREATE_PROJECT_OPTIONS.map { |opt| opt.name.to_sym }
  phraseapp_opts = options.slice(*phraseapp_create_options)
  ...
end

Not sure if this is unorthodox or not though. I tested it and works as expected. Let me know if there's a way I can finesse this. Otherwise I think it's gtg.

[chrisandreae]

I'd say the code is a little surprising to read because the constant array looks like you're just preparing pure values, but actually each method_option() call is making a side-effect on the current class. It's entirely valid code, just unexpected.

I think it'd be less surprising to drive it along the lines of:

PHRASEAPP_OPTIONS = {
 zero_plural_form_enabled: {
   type: :boolean,
   desc: 'Displays the input fields for the \'ZERO\' plural form for every key as well although only some languages require the \'ZERO\' explicitly.',
  },
}

PHRASEAPP_OPTIONS.each do |name, params|
  method_option(name, **params)
end

[...]

def setup
  [...]
  phraseapp_opts = options.slice(*PHRASEAPP_OPTIONS.keys)
  [...]
end

but that's a bit of a nitpick.

[gordoncl]

I'd say the code is a little surprising to read because the constant array looks like you're just preparing pure values, but actually each method_option() call is making a side-effect on the current class. It's entirely valid code, just unexpected.

but that's a bit of a nitpick.

Nah, I agree with you rationale.

I think it'd be less surprising to drive it along the lines of:

Thanks for the code!

[gordoncl]

Any thoughts on how to keep this better synchronized with the current options?

[chrisandreae]

Extract the options into a separate constant structure, and drive both the method_options calls above and this processing from it?

[chrisandreae]

I think it seems pretty reasonable. The remaining question is just whether we want to pick and choose the Phrase options we want people to use, or if we try to allow anything.