Skip to content
This repository was archived by the owner on Dec 29, 2022. It is now read-only.

Remove workspace mode, change some options #858

Merged
merged 3 commits into from
May 11, 2018
Merged

Remove workspace mode, change some options #858

merged 3 commits into from
May 11, 2018

Conversation

nrc
Copy link
Member

@nrc nrc commented May 9, 2018

r? @Xanewok

Discussed in #132

Copy link
Member

@Xanewok Xanewok left a comment

Choose a reason for hiding this comment

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

Looks good, this simplifies the building routine!

However, if I understand correctly, build_bin and build_lib seem to be completely ignored after the change. Wouldn't we want to just remove them, instead?

(As a side note, it'd be hood to investigate recent AppVeyor failures with 'missing id field')

@emk
Copy link

emk commented May 9, 2018

Hello! A quick question: Does "remove workspace mode" mean "remove workspace support" (yikes) or "merge the workspace and non-workspace code into a single mode" (wonderful news)?

@Xanewok
Copy link
Member

Xanewok commented May 9, 2018

@emk Thankfully, the latter 😄

@nrc
Copy link
Member Author

nrc commented May 10, 2018

However, if I understand correctly, build_bin and build_lib seem to be completely ignored after the change. Wouldn't we want to just remove them, instead?

Yeah, they did nothing in workspace mode, so they still do nothing. I didn't remove them in case we want to add them back (or handle gracefully with an error message or something).

As a side note, it'd be hood to investigate recent AppVeyor failures with 'missing id field'

Have you seen this a lot? I saw it once when working on this PR, wasn't sure if it was new or not.

@Xanewok
Copy link
Member

Xanewok commented May 11, 2018

Have you seen this a lot? I saw it once when working on this PR, wasn't sure if it was new or not.

I saw this once before, probably Windows tests need adjusting after #856?

@Xanewok
Copy link
Member

Xanewok commented May 11, 2018

The Windows tests probably need to be adjusted, but that's not related to this PR. Thanks for cleaning it up!

@Xanewok Xanewok merged commit 5995f20 into master May 11, 2018
@Xanewok Xanewok deleted the workspace-mode branch June 30, 2019 15:57
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.

3 participants