Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge upstream changes #110

Merged
merged 157 commits into from
May 7, 2021
Merged

Merge upstream changes #110

merged 157 commits into from
May 7, 2021

Conversation

Zeitsperre
Copy link
Collaborator

@Zeitsperre Zeitsperre commented May 5, 2021

Overview

This PR fixes #99

Changes:

  • Integrated changes from upstream master, including re-enabling tox
  • Restored removed documentation

Related Issue / Discussion

Re-enabling tox is particularly high in the priorities list, given the changes that have come to pass with Travis CI. At the very least, we now know the risks associated with tying ourselves to a specific service API.

There are quite a lot of changes in here. Even though I performed a rebase-type merge to gradually solve the merge conflicts (time-consuming), there is likely a lot here to unpack. Some of my goals were as follows:

  • Make pytest optional (selecting against it removes much of the pytest-based code we have).
  • Leverage tox where possible and have CI services rely more on makefile recipes.
  • Restore most of the documentation that was previously removed so users/developers know how to contribute here.

Additional Information

#99
Ouranosinc/xclim#509

pyup-bot and others added 30 commits February 10, 2018 14:10
…e-4.4.2-to-4.5.1

Update coverage to 4.5.1
… pytest to 3.4.1 audreyfeldroy#412, update sphinx to 1.7.0 audreyfeldroy#407, removed unnecessary requirements audreyfeldroy#396
…ache

added `.pytest_cache/` to .gitignore
Per twoscoops/Creating-and-Distributing-Python-Packages#16, we've added .pytest_cache so this isn't added to a git repo.
@Zeitsperre Zeitsperre added the enhancement New feature or request label May 5, 2021
@Zeitsperre Zeitsperre requested review from cehbrecht and tlvu May 5, 2021 17:13
@Zeitsperre Zeitsperre self-assigned this May 5, 2021
@Zeitsperre Zeitsperre mentioned this pull request May 5, 2021
@tlvu
Copy link

tlvu commented May 5, 2021

I performed a rebase-type merge to gradually solve the merge conflicts (time-consuming)

Sorry to hear that it was time-consuming. I would suggest you merge instead of rebase. When you merge and something goes wrong, you still have the previous state before the merge to compare against or to revert back to. When you rebase, you lose that previous state and trying to go back will be even more time-consuming.

@Zeitsperre
Copy link
Collaborator Author

@tlvu I did end up making a merge commit instead, as rebasing was a nightmare. In any case, it seems to be working and now we have functional tests! I'm not sure how much of a change this will translate into vis-à-vis our projects. Thoughts?

@cehbrecht
Copy link
Member

@Zeitsperre just made a quick check building babybird from cookiecutter. Only comments:

Could also be done by another PR (at least for github ci) ... this PR has already many changes.

@Zeitsperre
Copy link
Collaborator Author

Thanks for the review @cehbrecht! I'm going to wait for @tlvu as well before anything. You're right that there's a lot going on here, so I'll add those to the next PR and afterwards we should be more or less in sync with upstream.

@tlvu
Copy link

tlvu commented May 6, 2021

Yes sorry @Zeitsperre will try to finish this review tonight or tomorrow.

Copy link

@tlvu tlvu left a comment

Choose a reason for hiding this comment

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

LGTM. Minor remarks, you can ignore if needed.

The expensive question is have you tried to cruft update this with our existing birds to test for a smooth transition?

I suggest merge this PR, then open a new PR for anything you find when trying to cruft update with existing birds. Maybe one bird at a time, then merge after each bird, so the "fix PR" so not stay open for too long. Also do not have to refresh all birds at once.

cookiecutter.json Show resolved Hide resolved
cookiecutter>=1.6.0
pytest-cookies==0.5.1
alabaster==0.7.12
watchdog==0.9.0
Copy link

Choose a reason for hiding this comment

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

Oh la pinning of dependencies. We'll have keep up-to-date with upstream often to get newer versions.

{{cookiecutter.project_slug}}/docs/source/index.rst Outdated Show resolved Hide resolved
@cehbrecht
Copy link
Member

@Zeitsperre feel free to merge when ready.

@Zeitsperre Zeitsperre merged commit d353f26 into master May 7, 2021
@Zeitsperre Zeitsperre deleted the upstream-changes_redo branch May 7, 2021 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement tox for easier automated processing