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

Fix #5729 #6352

Merged
merged 1 commit into from
Aug 21, 2019
Merged

Fix #5729 #6352

merged 1 commit into from
Aug 21, 2019

Conversation

gdziadkiewicz
Copy link
Contributor

No description provided.

@dsyme
Copy link
Contributor

dsyme commented Mar 21, 2019

Great! Link to the bug: #5729

@dnfclas
Copy link

dnfclas commented May 20, 2019

CLA assistant check
All CLA requirements met.

@cartermp
Copy link
Contributor

cartermp commented May 20, 2019

@gdziadkiewicz Are you seeking a review? If so, if you could:

  • Sign the .NET CLA
  • Mark this PR as ready for review

Then we can take a look.

@gdziadkiewicz
Copy link
Contributor Author

@cartermp It is still work in progress. I was recently busy and even managed to forget about this PR, but I will make it review ready this week.

@cartermp
Copy link
Contributor

@gdziadkiewicz No rush!

@gdziadkiewicz gdziadkiewicz force-pushed the feature/Fix_#5729 branch 2 times, most recently from 88957eb to e5ba86c Compare June 15, 2019 17:52
@gdziadkiewicz gdziadkiewicz marked this pull request as ready for review June 15, 2019 18:57
@gdziadkiewicz gdziadkiewicz force-pushed the feature/Fix_#5729 branch 2 times, most recently from 74b1818 to 2700c96 Compare August 19, 2019 20:56
@realvictorprm
Copy link
Contributor

You pushed 0 commits? Github bug lol

@realvictorprm
Copy link
Contributor

@gdziadkiewicz pushed 0 commits.

@cartermp
Copy link
Contributor

@realvictorprm It's possible to push empty commits, which will re-trigger CI.

@gdziadkiewicz
Copy link
Contributor Author

@cartermp Are there any plans to fix this CI error? It seems to be also present on master ATM
image

@KevinRansom
Copy link
Member

@brettfo

@cartermp
Copy link
Contributor

@gdziadkiewicz Yes, we have an internal email thread going with infrastructure folks. It doesn't appear to be related to our codebase.

@brettfo
Copy link
Member

brettfo commented Aug 19, 2019

@gdziadkiewicz The issue is present on all Windows jobs trying to restore FSharp.Core 4.6.2. There is no issue if it's trying to restore 4.7.0 (like in the release/dev16.3 branch) nor is there an issue on non-Windows machines. I'm having some internal conversations to try and figure out why this package isn't getting restored.

I have a temporary workaround that I'll be creating a PR for soon

@brettfo
Copy link
Member

brettfo commented Aug 19, 2019

#7424 should fix the issue. I'll watch and merge that one as soon as it's ready.

Once that's in you may need to rebase or close/open this PR to trigger a new merge and build.

@brettfo
Copy link
Member

brettfo commented Aug 20, 2019

#7424 is now in; you should be able to rebase (or otherwise force a re-merge) and get the tests to do something sane.

@gdziadkiewicz
Copy link
Contributor Author

Thanks @brettfo , now it builds fine on the build server. @cartermp From my perspective this is ready to be reviewed.

Copy link
Contributor

@cartermp cartermp left a comment

Choose a reason for hiding this comment

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

Change seems reasonable. @dsyme?

@cartermp
Copy link
Contributor

Give that FSharp.Core 4.7 is locked, this will be in F# vNext

@cartermp cartermp merged commit 3471e1c into dotnet:master Aug 21, 2019
@gdziadkiewicz gdziadkiewicz deleted the feature/Fix_#5729 branch August 21, 2019 18:16
nosami pushed a commit to xamarin/visualfsharp that referenced this pull request Feb 23, 2021
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.

7 participants