Skip to content

Add context to result #22

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jun 3, 2018
Merged

Add context to result #22

merged 4 commits into from
Jun 3, 2018

Conversation

hackebrot
Copy link
Owner

Resolve #20

@hackebrot hackebrot added the enhancement New feature or enhancement for pytest-cookies label Apr 27, 2018
@hackebrot hackebrot requested a review from michaeljoseph April 27, 2018 10:02
@hackebrot hackebrot mentioned this pull request Apr 27, 2018
Closed
cblegare
cblegare approved these changes May 4, 2018
View reviewed changes
Copy link

@cblegare cblegare left a comment

Choose a reason for hiding this comment

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

Bravo!

pytest_cookies.py
no_input=True,
)

# Run cookiecutter to generate a new project
project_dir = cookiecutter(
template,
no_input=True,
extra_context=extra_context,
Copy link

Choose a reason for hiding this comment

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

I am not sure what difference it would make, but I chose to pass the complete rendered context here. You are probably right to let cookiecutter render it again on it's own without overriding every single value.

@cblegare
Copy link

cblegare commented May 4, 2018 •
edited
Loading

some tests failed in CI 😞

I am not familiar with your CI setup nor with that testdir.runpytest contraption of yours for running tests within tests, but these look like false positives to me.

@hackebrot hackebrot mentioned this pull request May 4, 2018
Merged
@hackebrot
Copy link
Owner Author

I created a PR against Cookiecutter to address, what I feel might be the source of the test issues.

@hackebrot hackebrot force-pushed the add-context-to-result branch from c97223e to 23699d4 Compare May 4, 2018 20:39
@hackebrot
Copy link
Owner Author

What do you think @michaeljoseph?

michaeljoseph
michaeljoseph approved these changes May 7, 2018
View reviewed changes
Copy link
Collaborator

@michaeljoseph michaeljoseph left a comment

Choose a reason for hiding this comment

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

lgtm!

@hackebrot hackebrot merged commit 443ad61 into master Jun 3, 2018
@hackebrot hackebrot deleted the add-context-to-result branch June 3, 2018 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michaeljoseph michaeljoseph michaeljoseph approved these changes

@cblegare cblegare cblegare approved these changes

Assignees
No one assigned
Labels
enhancement New feature or enhancement for pytest-cookies
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Testing against samples of provided values, lots of them
3 participants
@hackebrot @cblegare @michaeljoseph