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

Added request name to results variable passed to report (#354) #390

Merged
merged 4 commits into from Jun 11, 2021

Conversation

hebertjulio
Copy link
Member

@hebertjulio hebertjulio commented Jun 7, 2021

Description

Show request names in the report.

Motivation behind this PR?

Requests for different purposes with the same paths.

What type of change is this?

Feature

Checklist

  • I have added a changelog entry / my PR does not need a new changelog entry. Instructions.
  • I have added/updated unit tests. Instructions.
  • New and existing unit tests pass locally with my changes. Instructions
  • I have self-documented code my changes by adding docstring(s) and comment(s). Instructions
  • Current PR does not significantly decrease the code coverage and docstring coverage.
  • My code follows the style guidelines of this project.
  • I have run ScanAPI locally and manually tested my changes. Instructions.

Issue

Closes #354

Copy link
Member

@camilamaia camilamaia left a comment

Choose a reason for hiding this comment

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

Awesome PR, thanks @hebertjulio !! I left some minor suggestions.

tests/unit/tree/request_node/test_run.py Outdated Show resolved Hide resolved
scanapi/tree/request_node.py Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@camilamaia camilamaia requested a review from Pradhvan June 8, 2021 17:21
@camilamaia
Copy link
Member

Screenshot to show how cool is the report with the change:

image

Thanks @hebertjulio !

Copy link
Member

@camilamaia camilamaia left a comment

Choose a reason for hiding this comment

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

@camilamaia
Copy link
Member

@Pradhvan can I go ahead and merge the PR? Or do you have any suggestions/comments?

@hebertjulio
Copy link
Member Author

I will resolve merge conflicts

@Pradhvan
Copy link
Member

@hebertjulio left two minor reviews. The rest of the PR looks good to me. Great job 🤗

scanapi/tree/request_node.py Outdated Show resolved Hide resolved
scanapi/tree/request_node.py Outdated Show resolved Hide resolved
@camilamaia
Copy link
Member

camilamaia commented Jun 11, 2021

@hebertjulio @Pradhvan I am going to release a new version right now and I would love to have already this new feature on it. Can I add Pradhvan's suggestions and merge it? If we need more changes in the docstrings, we can open a new PR later. Is that ok?

@camilamaia camilamaia requested a review from Pradhvan June 11, 2021 12:29
@camilamaia camilamaia merged commit 781593d into scanapi:master Jun 11, 2021
@camilamaia camilamaia mentioned this pull request Jun 11, 2021
@hebertjulio
Copy link
Member Author

Of course @camilamaia , that's fine with me, thanks 😄

@hebertjulio hebertjulio deleted the 354 branch June 11, 2021 15:03
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.

Add request name to results variable passed to report
3 participants