Skip to content

A 'fix' for changing context on subsequent generations #262

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jacob7395
Copy link
Contributor

@jacob7395 jacob7395 commented Aug 29, 2024

An draft PR with a possible solution to #261

}

[Fact]
public void Generate_RulesSet_Should_Generate_Wtih_Ruleset_And_Oeverride()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just or my testing, not sure how to name a test that checks for this issue I would go with

Genrate_ShouldUserCorrectRuleset_WhenRulestChangesInSubsiquentCalles

But that is a different style

PrepareFinish(context);
CreateGenerateContext(ruleSets);

PrepareCreate();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could do both these methods as part of class construction since the context is no longer required.

@jacob7395 jacob7395 changed the title A 'fixed' of tracking the context A 'fix' for changing context on subsequent generations Aug 29, 2024
soenneker
soenneker previously approved these changes Sep 11, 2024
Copy link
Owner

@soenneker soenneker left a comment

Choose a reason for hiding this comment

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

Automatically approved.

@soenneker soenneker dismissed their stale review September 11, 2024 14:18

Accidental approval, sorry, will get to this shortly

@jacob7395
Copy link
Contributor Author

@soenneker no worries, it's more of a suggestion of how it could be fixed. I don't like how coupled it is to the underlying implementation of Bogus. If they alter how the count it indexed could brake in weirder ways.

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