Skip to content

Conversation

@jsa34
Copy link
Collaborator

@jsa34 jsa34 commented Sep 8, 2024

Going a step further than my previous PR that simply parsed the feature files using the official parser, but then copied this data into the existing parser objects, this PR removes the old parser completely and either uses the data directly that is parsed into the models from the Gherkin parser or adds needed functionality to the Gherkin parser models to enhance just being DAOs for pytest-bdd to use the data as it needs to.

@jsa34
Copy link
Collaborator Author

jsa34 commented Sep 8, 2024

I'm thinking a different approach might be better to keep the DAO/mapping and logic encapsulated

@youtux youtux marked this pull request as ready for review September 11, 2024 19:38
@sourcery-ai
Copy link

sourcery-ai bot commented Sep 11, 2024

🧙 Sourcery has finished reviewing your pull request!


Tips
  • Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
  • Continue your discussion with Sourcery by replying directly to review comments.
  • You can change your review settings at any time by accessing your dashboard:
    • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
    • Change the review language;
  • You can always contact us if you have any questions or feedback.

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

We've reviewed this pull request using the Sourcery rules engine. If you would also like our AI-powered code review then let us know.

@youtux youtux marked this pull request as draft September 11, 2024 19:38
Copy link
Contributor

@youtux youtux left a comment

Choose a reason for hiding this comment

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

let's first get #698 merged, then let's focus on this.

Actually, could you rebase this on top of #698, so that in GitHub I can check the diff from that only?

@jsa34 jsa34 closed this Sep 12, 2024
@jsa34
Copy link
Collaborator Author

jsa34 commented Sep 12, 2024

Closed to focus on parser, and ended up just dumping things I was trying on this branch - apologies!

@jsa34 jsa34 mentioned this pull request Sep 22, 2024
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.

2 participants