Skip to content

Conversation

spookylukey
Copy link
Collaborator

We don't actually depend on anything that was added after 0.10, and a tight check gives problems with other tools (e.g. compare-locales is constrained to '>=0.10,<0.11').

@spookylukey spookylukey requested a review from Pike March 11, 2019 16:32
Copy link
Contributor

@Pike Pike left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't manage to find why I bumped the syntax. Maybe I wanted to use visitors and didn't end up doing so?

I'm wondering, if we accept more revisions, should we also test them? I don't think we need to do all versions and platforms, but maybe min-version on one? Which also leaves me wondering if relying on compat ranges to specify what we're testing was right. Maybe we should have specified an exact revision instead of dropping the range?

@spookylukey
Copy link
Collaborator Author

Specifying exact numbers for tox/travis has the advantage of being more deterministic. It can be annoying when Travis fails for a PR (or a push) because of a problem with a new version of a dependency, not because of anything that was changed.

On the other hand, such failures do alert you to what will actually happen for new users if they try to install the library.

Perhaps the best would be:

  • Pin tests to specific versions
  • Add any other version combinations we think might be problematic
  • Possibly add a 'latest' combination that uses specifies only minimum version, and in Travis put that in allow_failures

@Pike
Copy link
Contributor

Pike commented Mar 12, 2019

Sounds great, thanks for bearing with me thinking aloud on this PR.

@spookylukey spookylukey force-pushed the better_fluent_syntax_dep branch from 7741444 to d3d8734 Compare March 14, 2019 09:41
@spookylukey
Copy link
Collaborator Author

OK, this looks good to go now. We've got pinned version testing in tox and Travis, against all Python versions, with a few additional ones:

  • one run testing fluent.syntax==0.10
  • one run testing the latest of everything (what you get if you just do 'pip install fluent.runtime' with nothing else already installed).

I've checked that Travis is actually installing what I intended, and it is (apart from some cases where exact dependencies are not pinned, where it seems that 'six' and 'attrs' are already installed, and I'm not sure at what version).

After a bit of wrestling with tox-travis to do all this and failing, I removed it, so this also addresses #89, and overall it is better and simpler, even if it requires a bit of duplication.

Are we good to do on this now?

@spookylukey spookylukey merged commit 3cc4ecd into projectfluent:master Apr 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants