Skip to content
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.

Sign up for GitHub

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

Jump to bottom

build static before shared #468

Merged
merged 2 commits into from
Oct 30, 2020
Merged

build static before shared #468

merged 2 commits into from
Oct 30, 2020

Conversation

ericLemanissier
Copy link
Contributor

usually, the static build is the default one

  • Refer to the issue that supports this Pull Request.
  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.

@ericLemanissier
Copy link
Contributor Author

Before I go spend time fixing unit tests, What do you think of this change @uilianries ?

@uilianries
Copy link
Member

I don't see any advantage. Why do you need that, besides the fact shared=False is the default for most of recipes?

@ericLemanissier
Copy link
Contributor Author

It's not "needed" per se.it makes the default config available earlier, which allows starting dependants earlier, especially in case of network error/ci timeout etc.

@uilianries
Copy link
Member

Okay, go ahead. I gonna work on CPT today and tomorrow, we can include it together.

Croydon
Croydon approved these changes Oct 27, 2020
View reviewed changes
Copy link
Contributor

@Croydon Croydon left a comment

Choose a reason for hiding this comment

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

LGTM

I think it makes sense if we mimic the build order of CCI as close as possible. This is a step towards this

ericLemanissier and others added 2 commits October 27, 2020 22:12
@ericLemanissier
build static before shared
4fc377e 
usually, the static build is the default one
@ericLemanissier
fix unit tests
ed5b021
@uilianries
Copy link
Member

Yes, thanks for pinging. I forgot this one, let's continue this week.

@uilianries uilianries closed this Oct 27, 2020
@uilianries uilianries reopened this Oct 27, 2020
uilianries
uilianries approved these changes Oct 30, 2020
View reviewed changes
Copy link
Member

@uilianries uilianries left a comment

Choose a reason for hiding this comment

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

LGTM

@uilianries uilianries merged commit ab246f0 into conan-io:develop Oct 30, 2020
@ericLemanissier ericLemanissier deleted the patch-2 branch October 30, 2020 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danimtb danimtb danimtb approved these changes

@uilianries uilianries uilianries approved these changes

@Croydon Croydon Croydon approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ericLemanissier @uilianries @Croydon @danimtb