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

Document JsonApiEndpoints enum #1301

Closed
wants to merge 9 commits into from
Closed

Document JsonApiEndpoints enum #1301

wants to merge 9 commits into from

Conversation

verdie-g
Copy link
Contributor

Closes #1298

QUALITY CHECKLIST

@codecov
Copy link

codecov bot commented Aug 20, 2023

Codecov Report

Merging #1301 (d7962b7) into master (68e1854) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1301   +/-   ##
=======================================
  Coverage   93.03%   93.03%           
=======================================
  Files         268      268           
  Lines        8795     8795           
=======================================
  Hits         8182     8182           
  Misses        613      613           

@bkoelman
Copy link
Member

Thanks. I'll get back to this next month, after my break.

Copy link
Member

@bkoelman bkoelman left a comment

Choose a reason for hiding this comment

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

Please note the line at the top of this file:

// IMPORTANT: An internal copy of this type exists in the SourceGenerators project. Keep these in sync when making changes.

These changes should be replicated in the other file.

@verdie-g verdie-g requested a review from bkoelman September 3, 2023 20:15
@verdie-g
Copy link
Contributor Author

verdie-g commented Sep 4, 2023

The CI error is weird, I did run cleanupcode 🤔

@bkoelman
Copy link
Member

bkoelman commented Sep 4, 2023

The CI error is weird, I did run cleanupcode 🤔

Strange indeed. Running locally on Windows does not produce a diff. I haven't seen this happen before, will investigate. What OS are you using?

@verdie-g
Copy link
Contributor Author

verdie-g commented Sep 4, 2023

Windows 11 and

$ dotnet --info
.NET SDK:
 Version:   7.0.400
 Commit:    73bf45718d

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.22621
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\7.0.400\

Host:
  Version:      7.0.10
  Architecture: x64
  Commit:       a6dbb800a4

.NET SDKs installed:
  6.0.413 [C:\Program Files\dotnet\sdk]
  7.0.400 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 6.0.21 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 7.0.10 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 6.0.21 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 7.0.10 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 6.0.21 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 7.0.10 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

Other architectures found:
  None

Environment variables:
  Not set

global.json file:
  Not found

@verdie-g
Copy link
Contributor Author

verdie-g commented Sep 4, 2023

Could it be a line-ending thing? Maybe locally I'm using CRLF and the CI is using LF the number of characters on the line is different?

@bkoelman
Copy link
Member

bkoelman commented Sep 7, 2023

Could it be a line-ending thing? Maybe locally I'm using CRLF and the CI is using LF the number of characters on the line is different?

Yes, but I don't think that should matter. It has always worked that way. I've created a temporary PR #1304 to investigate.

@bkoelman
Copy link
Member

@verdie-g Sorry this is taking so long. I've opened a bug at https://youtrack.jetbrains.com/issue/RSRP-494002, but it hasn't been triaged yet.

Meanwhile, can you please merge or rebase on the latest changes in master? I've replaced AppVeyor with GitHub Actions. This is a nice opportunity to test whether everything works from a community PR. I can't do it myself because my account has write permissions and I'd like to confirm builds work without that. Sadly, it won't fix the cleanupcode bug though.

@verdie-g
Copy link
Contributor Author

Looks like it's failing for macos

@bkoelman
Copy link
Member

Looks like it's failing for macos

Yes, same as linux.

@verdie-g verdie-g closed this by deleting the head repository Nov 9, 2023
@bkoelman
Copy link
Member

bkoelman commented Nov 9, 2023

@verdie-g I'd like to keep this PR open. I suspect this was closed unintentionally as a side effect of deleting your fork. I'm unable to reopen because your fork no longer exists. Can you restore it?

We still want to take this PR. The related issue has been triaged with priority critical, so I have high hopes a fix will become available soon.

@verdie-g
Copy link
Contributor Author

verdie-g commented Nov 9, 2023

I completely forgot about that PR. I'll try restoring the fork when I get some time.

@verdie-g verdie-g mentioned this pull request Dec 14, 2023
4 tasks
@bkoelman
Copy link
Member

Superseded by #1423.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

JsonApiEndpoints documentation
2 participants