Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Comments

Close side panel

Tests - Automatic Path Finding and Remove Environment Variables#62

Merged
liamnichols merged 4 commits intoCreateAPI:mainCreateAPI/CreateAPI:mainfrom
LePips:automatic-path-finding-for-testsLePips/CreateAPI:automatic-path-finding-for-testsCopy head branch name to clipboard
Jul 28, 2022
Merged

Tests - Automatic Path Finding and Remove Environment Variables#62
liamnichols merged 4 commits intoCreateAPI:mainCreateAPI/CreateAPI:mainfrom
LePips:automatic-path-finding-for-testsLePips/CreateAPI:automatic-path-finding-for-testsCopy head branch name to clipboard

Conversation

@LePips
Copy link
Contributor

@LePips LePips commented Jul 28, 2022

#filePath is the closest we can get in regards to "automatic file path finding".

Tests/CreateAPITests/Snapshots.swift Outdated Show resolved Hide resolved
@LePips
Copy link
Contributor Author

LePips commented Jul 28, 2022

Is there a specific reason why an environment variable is set to generate versus a static bool? I think it would be more convenient if it was just a static bool for general development and then a FORCE_GENERATE_SNAPSHOT environment variable would override that to force generation elsewhere.

Copy link
Member

@liamnichols liamnichols left a comment

Choose a reason for hiding this comment

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

Oh awesome! Thanks so much for picking this up 🙌

Is there a specific reason why an environment variable is set to generate versus a static bool? I think it would be more convenient if it was just a static bool for general development and then a FORCE_GENERATE_SNAPSHOT environment variable would override that to force generation elsewhere.

Not that I'm aware of. I agree though, a static bool is easier to conditionally enable I think so happy to change it (same with the OPEN_DIFF var)

Tests/CreateAPITests/Snapshots.swift Outdated Show resolved Hide resolved
Tests/CreateAPITests/Snapshots.swift Outdated Show resolved Hide resolved
@LePips
Copy link
Contributor Author

LePips commented Jul 28, 2022

I'll be able to provide steps for testing after, or during, your README update PR as mentioned in your current PR #52.

Tests/CreateAPITests/Snapshots.swift Outdated Show resolved Hide resolved
@LePips LePips requested a review from liamnichols July 28, 2022 15:27
@LePips LePips changed the title Automatic Path Finding for Tests Tests - Automatic Path Finding and Remove Environment Variables Jul 28, 2022
Copy link
Member

@liamnichols liamnichols left a comment

Choose a reason for hiding this comment

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

Looking great! Thanks for sorting the environment variables too 🙏

I think we should prefer private over fileprivate when at the top-level but other than that, everything seems great 👍

Tests/CreateAPITests/Snapshots.swift Outdated Show resolved Hide resolved
Tests/CreateAPITests/Snapshots.swift Outdated Show resolved Hide resolved
Tests/CreateAPITests/Snapshots.swift Outdated Show resolved Hide resolved
Copy link
Member

@liamnichols liamnichols left a comment

Choose a reason for hiding this comment

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

Awesome stuff! Thanks for doing this, it makes it so much easier to test locally 🙇

@liamnichols liamnichols merged commit 7520817 into CreateAPI:main Jul 28, 2022
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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.