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.

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

Retry project cracking when there isn't a targetPath #613

Merged
merged 6 commits into from
Oct 25, 2020

Conversation

wallymathieu
Copy link
Member

@wallymathieu wallymathieu commented Oct 13, 2020

I've based the logic on this comment:
https://github.com/alfonsogarciacaro/TypeProviderTest/blob/4cb6442afce6db707e56a1a43c79d3cc7cd4211a/ASTViewer/ProjectCoreCracker.fs#L111-L114

      // As workaround, lets choose the first of the target frameworks and use that

@wallymathieu wallymathieu requested a review from dsyme October 13, 2020 11:03
@dsyme
Copy link
Contributor

dsyme commented Oct 15, 2020

Fix conflicts with master please?

@wallymathieu
Copy link
Member Author

Will do.

@wallymathieu
Copy link
Member Author

How does it look now @dsyme ?

@dsyme
Copy link
Contributor

dsyme commented Oct 24, 2020

big conflict here, with #618 , my apologies

@dsyme
Copy link
Contributor

dsyme commented Oct 24, 2020

big conflict here, with #618 , my apologies

I should have prioritised this one since you've already fixed conflicts one :-(

@wallymathieu
Copy link
Member Author

Does it look OK @teo-tsirpanis ?

@teo-tsirpanis
Copy link
Contributor

Yes @wallymathieu, it seems OK to me.

BTW most of it will be rewritten anyway when ionide/proj-info#87 is fixed.

@wallymathieu
Copy link
Member Author

Sounds good. So then some of this can be removed once that happens 🙂?

@teo-tsirpanis
Copy link
Contributor

Yep. But let's merge it for now...

@wallymathieu
Copy link
Member Author

big conflict here, with #618 , my apologies

I should have prioritised this one since you've already fixed conflicts one :-(

The conflict was kind of small in the sense that it was mostly moved code.

@wallymathieu
Copy link
Member Author

I'll merge it @dsyme later on today.

@wallymathieu wallymathieu merged commit 95222de into fsprojects:master Oct 25, 2020
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.

3 participants