Hide main functionality behind a flag

[ryanhiebert]

I'm debating whether it would be wise to require the detection to be behind a flag, the same way that after all is behind a flag, and that any other feature would also be behind a flag. Magic is best used sparingly and judiciously.

There are some places that I think magic is the only thing that makes sense. In particular, our hacks, of which there is currently only one, that allows the pypy3 pre-releases to work on Travis. There's no sense in doing that less magically.

The argument for the general detection behavior being behind a flag, however, is that it allows tox to run the same on Travis as it does locally. It's easy enough to add a flag in tox_addoption. Explicit opt-in allows users to decide exactly what they do (and don't) want overridden.

A future case where this might play out is if someone wishes to Tox-Travis for its ability to report back to GitHub (#73), but want to do all of that in a single job. It really shouldn't require a bunch of extra configuration work to do what Tox does by default.

An alternative would be a flag to disable detection. That would be a backward compatible change, and would give the needed functionality, but it would have the flag being backward from rest of the features, which are (and are going to be) opt-in.

[rpkilby]

Two thoughts:

• This flag would obviously conflict with [travis:env] use, and would need to raise a configuration exc.
• A disable flag seems acceptable, since this sounds like a minority case.

[ryanhiebert]

This flag would obviously conflict with [travis:env] use, and would need to raise a configuration exc.

I'm not sure what you mean. The flag would hide basically all of the current functionality that gets configured in the [travis] and the [travis:env] sections, to make it opt-in. Nothing would change or break, apart from adding a flag (--detect or --auto-envlist).

A disable flag seems acceptable, since this sounds like a minority case.

It certainly would be a minority case currently. If you're not using this feature in the current package, there's really nothing less. That's why I brought up the future feature of reporting jobs to GitHub. That's one case that I can readily see a desire to have Tox work as it does normally, and just use a particular feature that we're adding to it. A positive flag would keep feature usage consistent.

If the negative flag is preferred, and likely even if it is not, I think a --no-detect or --no-auto-envlist is the appropriate spelling.

[ryanhiebert]

The current flag is --travis-after. So something like --travis-detect, --travis-envlist, --travis-detect-envlist would be appropriate (and/or the negative). --travis-envlist seems fine to me, which would make naming the files envlist in #83 just fine.

---

There's one more case that I just thought of to consider, and that's if we wanted to add options to match these flags in the [travis] config section. I can think of two ways that might look:

[travis]
envlist = False
after = True

This makes sense, but conflicts with an idea that's been floated before of using the envlist key as an override for the global envlist from Tox, instead of just detecting the envs that tox knows about. I can't think of a better spelling for that use, or else I'd be happy to move forward with this one.

[travis]
features =
  envlist: False
  after: True

This solves the conflicting key problem, but I think it looks ugly, and it would be more annoying to parse.

What do you think?

[rpkilby]

Right, I was saying flag but thinking config option. I'd argue that the config options makes more sense:

• users are not using these flags locally
• users are not calling these flags on some CI builds but not others

As to features, maybe something like:

[travis]
features = 
    envlist
    after

• should be a list where presence implies opt-in
• should be a multi line list, split on commas/whitespace
• should default to just envlist

Additionally, it might be helpful if feature mismatch printed a warning in the tox output (both local and CI).

[travis]
features =
    after

[travis:env]
DJANGO =
    1.8: django18
    1.11: django111

[travis:after]
travis = python: 3.6
env = DJANGO: 1.11

In the above, we could notify the user that they've provided a [travis:env] selection, but haven't opted into that feature with the overridden features list.

[ryanhiebert]

users are not calling these flags on some CI builds but not others

In the typical case, I believe you're right. However, the build for tox-travis, for example, runs the tests twice for each matrix job. Once with the sdist, and the other with the wheel. Because of that, we only want to wait on after on the second tox run (using the wheel). This is a use-case that I find quite valid and want to continue to support through CLI flags.

[ryanhiebert]

I'm concluding this issue by thinking that with feature flags we'll want to have pairs. For this one, I'd like the flags to be spelled --travis-envlist and --no-travis-envlist, with this feature being the only one that is enabled by default as a special case.

If in the future we wish to allow the list of active features to be configured in the config file, which I'd be fine with but probably would not use myself, I'd want it to be as @rpkilby described above, with envlist being the default for the features setting, and any override needing to specify envlist to keep it working. I don't think that this will necessarily require any warning.

The feature names should match the flags, so --travis-envlist would match the envlist feature, and --travis-after would match the after feature. Any new features should have similar pairings in this idea. Thanks for helping think about it enough to get away from it being a sticking point in the continued work on #83 .