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

[INFRA] bump up mkdocs-materials version #211

Merged
merged 2 commits into from
Apr 25, 2019

Conversation

sappelhoff
Copy link
Member

@sappelhoff sappelhoff commented Apr 24, 2019

closes #208

This PR is a 2nd attempt to solve the vulnerability issue arising from Jinja2 versions below 2.10.1. Specifically, I am trying to update the Pipfile.lock through pipenv instead of manual edits.

This was my workflow:

  1. cd bids-specification
  2. pip install pipenv ... can be done in any of your environments
  3. pipenv install to make an environment for our repository
  4. pipenv shell to activate the env
  5. do whatever upgrading you want ... e.g., pip install -U Jinja2 (upgrade Jinja2)
  6. pipenv lock to lock the current package versions in the Pipfile.lock file
  7. git add and commit the changed Pipfile.lock and push

Note that I also updated our mkdocs-material version and this lead to upgrades in a lot of our packages.

I have no clue, why the "Markers" disappeared from the Pipfile.lock, but I don't think we need them.

See the rendered spec here: https://518-150465237-gh.circle-artifacts.com/0/home/circleci/project/site/01-introduction.html

Copy link
Collaborator

@effigies effigies left a comment

Choose a reason for hiding this comment

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

This looks reasonable. And I prefer this process to manual patches, as I think it's less likely that we'll end up with an unsatisfiable dependency.

On the other hand, it does introduce the possibility of changes in the output HTML that would be hard to detect without close inspection of each artifact. It would be nice to try to figure out a way to compare before-and-after HTML to target visual inspections, but that does not need to be addressed here.

Copy link
Collaborator

@franklin-feingold franklin-feingold left a comment

Choose a reason for hiding this comment

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

This looks good. I'll close my PR. The rendered artifact looked good, not sure what the markers affected (as you pointed out they were deleted from this re-lock). Curiously the hashes generated by pipenv for jinja is the same as the one I found in another repo. Perhaps the hashes are general for that version of the package?

@sappelhoff
Copy link
Member Author

Perhaps the hashes are general for that version of the package?

yes, these are the specific hashes, they should be the same everywhere. It's just more error-proof (and also easier once you know how) to update these automatically via pipenv instead of searching and editing manually

@sappelhoff
Copy link
Member Author

I am merging this now to fix the security alert.

@sappelhoff sappelhoff merged commit 257c32b into bids-standard:master Apr 25, 2019
@sappelhoff sappelhoff deleted the bump_mkdocs_material branch April 25, 2019 16:59
sappelhoff added a commit to sappelhoff/bids-specification that referenced this pull request Jul 16, 2019
@sappelhoff sappelhoff mentioned this pull request Jul 16, 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.

3 participants