-
Notifications
You must be signed in to change notification settings - Fork 130
Update csharpier: 0.30.6 -> 1.1.1 #369
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
Conversation
Apparently, the nix-build was not successful. I had to update the flake nixpkgs input to have access to the new csharpier version. This somehow seems to break deno, even though locally I cannot reproduce this (on both aarch64-darwin and x86_64-linux i can format files with deno-fmt without problems). Not sure how I can fix this. Anyone can shed some light on these errors? |
I also cannot repro the issue locally. Feels like a flaky build. @brianmcgee is it possible to re-trigger the build and see if it succeeds on a second try? |
@zimbatm |
Yes, can you dig into the errors and see where they are coming from? CI works well on the other branches, so I'm not sure what the root cause is |
This is the error I'm seeing for the aarch64-darwin failure on https://buildbot.numtide.com/#/builders/16/builds/383: cachix: ConnectionError (HttpExceptionRequest Request {
host = "cachix.org"
port = 443
secure = True
requestHeaders = [("Accept","application/json;charset=utf-8,application/json"),("Authorization","<REDACTED>"),("User-Agent","cachix 1.7.9")]
path = "/api/v1/cache/numtide"
queryString = ""
method = "GET"
proxy = Nothing
rawBody = False
redirectCount = 10
responseTimeout = ResponseTimeoutDefault
requestVersion = HTTP/1.1
proxySecureMode = ProxySecureWithConnect
}
(InternalException (HandshakeFailed (Error_Misc "Network.Socket.recvBuf: does not exist (No route to host)")))) To me, this looks like a network failure that is most likely completely unrelated to any changes in this PR. If a rebuild can be triggered, I suspect it would be successful. |
Head branch was pushed to by a user without write access
c4e4172
to
5e4f878
Compare
@zimbatm The commit still has the wrong name, should I rename it too? (the update is now to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thomaslaich Are any of the changes dependent on version 1.1.1 of csharpier specifically, or are they generic enough that they would support major version 1 generally? If this is support for major version 1, I would recommend updating the PR title and commit message to state that rather than mention a specific version.
I ask because the PR title mentions version 1.1.1, but the version of the csharpier
package in this repo's inputs.nixpkgs
appears to be 1.0.3. I checked the version via the following nix-repl interaction:
nix-repl> :lf .
Added 16 variables.
nix-repl> inputs.nixpkgs.outputs.legacyPackages.x86_64-linux.csharpier
«derivation /nix/store/li0lf21fw1gzicsvw5k7bgzj6dnv8sj0-csharpier-1.0.3.drv»
includes = [ | ||
"*.cs" | ||
"*.csproj" | ||
# "*.slnx" # add this with 1.0.3 release |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this line be uncommented now that the package is at an appropriate version?
All good. Feel free to send a follow-up PR if you want to address @lafrenierejm's comment (I don't have enough context to judge) |
Update
csharpier
to support breaking changes in v1.0.0The
csharpier
tool introduced breaking changes in its 1.0.0 release:dotnet-csharpier
tocsharpier
.format
, i.e.,csharpier format <file>
instead ofdotnet-csharpier <file>
.This PR updates the
csharpier
program definition intreefmt-nix
to use the new CLI interface.EDIT: I also added myself as the maintainer if that's ok.