Skip to content
This repository has been archived by the owner on Apr 30, 2024. It is now read-only.

Simplify features build #24

Merged
merged 4 commits into from
Feb 24, 2020
Merged

Simplify features build #24

merged 4 commits into from
Feb 24, 2020

Conversation

vbfox
Copy link
Contributor

@vbfox vbfox commented Feb 23, 2020

Note: this PR builds on #23 and provide a more feature-proof fix of the problem.

Motivation

The source of #23 is that there was code duplication so fixes were only applied to one of the variants.

This change move the conditional check for v1 & v2 in the builder inside the methods instead of duplicating them

Test Plan (required)

I ran both cargo build and cargo test with/without v1 & v2 (There are still a few unused things warning when only one of them is present but they build and pass tests)

@Mythra
Copy link
Collaborator

Mythra commented Feb 23, 2020

This needs to be rebased, and then I can take a look 😄 Thanks for answering part of my previous question heh.

@vbfox vbfox requested a review from Mythra February 23, 2020 16:59
@vbfox
Copy link
Contributor Author

vbfox commented Feb 23, 2020

@securityinsanity done

@Mythra
Copy link
Collaborator

Mythra commented Feb 23, 2020

Hey sorry can you rebase again? 😅 (no need to comment when you're done, I'll just checkback later today 😄 )

This was referenced Feb 24, 2020
@Mythra
Copy link
Collaborator

Mythra commented Feb 24, 2020

Thank you!

@Mythra Mythra merged commit 9483daa into instructure:master Feb 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants