Skip to content

run make upgrade to update jinja2#6

Merged
DawoudSheraz merged 5 commits intomasterfrom
dsheraz/prod-343
Jun 17, 2019
Merged

run make upgrade to update jinja2#6
DawoudSheraz merged 5 commits intomasterfrom
dsheraz/prod-343

Conversation

@DawoudSheraz
Copy link
Contributor

@DawoudSheraz DawoudSheraz commented Jun 13, 2019

Description: This PR updates the jinja2 to the latest version. Please see PROD-343 for more information.

JIRA: PROD-343

Dependencies: dependencies on other outstanding PRs, issues, etc.

Merge deadline: List merge deadline (if any)

Reviewers:

  • tag reviwer
  • tag reviewer

Merge checklist:

  • All reviewers approved
  • CI build is green
  • Version bumped
  • Changelog record added
  • Documentation updated (not only docstrings)
  • Commits are squashed
  • PR author is listed in AUTHORS

Post merge:

  • Create a tag
  • Check new version is pushed to PyPi after tag-triggered build is
    finished.
  • Delete working branch (if not needed anymore)

Author concerns: List any concerns about this PR - inelegant
solutions, hacks, quick-and-dirty implementations, concerns about
migrations, etc.

@DawoudSheraz DawoudSheraz requested a review from bmedx June 14, 2019 05:52
@DawoudSheraz
Copy link
Contributor Author

@bmedx Tagging you here for a quick code review as you were one of the suggestions for the reviewers.

@DawoudSheraz DawoudSheraz requested a review from awaisdar001 June 14, 2019 05:53
# This file is autogenerated by pip-compile
# To update, run:
#
# pip-compile --output-file requirements/travis.txt requirements/travis.in

Choose a reason for hiding this comment

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

Can you add the following in the Makefile to avoid getting ^^ message in each .txt file?
upgrade: export CUSTOM_COMPILE_COMMAND=make upgrade

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

include CONTRIBUTING.rst
include LICENSE.txt
include README.rst
include requirements/base.in

Choose a reason for hiding this comment

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

What is the role of adding / including the requirements file here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By including this file in the Manifest.in, the file will become available in the distribution package. It was needed as without it, the setup.py was not able to locate the requirements/base.in when defining install_requires through function.

@DawoudSheraz DawoudSheraz requested a review from awaisdar001 June 14, 2019 11:28
Copy link

@awaisdar001 awaisdar001 left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

Copy link
Contributor

@bmedx bmedx left a comment

Choose a reason for hiding this comment

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

I was able to make upgrade and make requirements without issues. 👍

@DawoudSheraz DawoudSheraz merged commit 69a0a0d into master Jun 17, 2019
@DawoudSheraz DawoudSheraz deleted the dsheraz/prod-343 branch June 17, 2019 06:49
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.

3 participants