Skip to content

Conversation

@Romfos
Copy link
Contributor

@Romfos Romfos commented Nov 29, 2025

Changes:

How to use:
image

If you want to publish directly to nuget owner of this repo should set NUGET_API_KEY to the repo secrets and check "publish to nuget" during release build

@Romfos Romfos marked this pull request as ready for review November 29, 2025 20:55
@Romfos
Copy link
Contributor Author

Romfos commented Nov 29, 2025

Maybe someone know - what is "Castle.Core.Tests.WeakNamed"? What is the purpose of that?

cc @stakx

@Romfos Romfos mentioned this pull request Nov 29, 2025
10 tasks
@stakx
Copy link
Member

stakx commented Nov 29, 2025

Sorry, I currently don't have sufficient capacity to look into migrating away from Appveyor if I also want to look at actually improving DynamicProxy. I'd rather spend some of my time there at the moment.

Getting rid of Appveyor will be a good thing... but hopefully it can wait a little longer.

Leaving this PR open to be reviewed at a later time (or by someone else).

Maybe someone know - what is "Castle.Core.Tests.WeakNamed"? What is the purpose of that?

DynamicProxy has to be able to deal with types both from strong-named ("signed") and non-strong-named ("unsigned") assemblies. Depending on whether a type to be proxied comes from one or the other, DynamicProxy needs to do slightly different things. Therefore we need both strong-named and non-strong-named assemblies in our test suite to cover all those cases. The Castle.Core.Tests.WeakNamed project targets the latter.

@Romfos
Copy link
Contributor Author

Romfos commented Nov 29, 2025

sad. As I See appveyor image doesn't have .NET 10 for now., This is blocked for this

@stakx
Copy link
Member

stakx commented Dec 9, 2025

appveyor image doesn't have .NET 10 for now., This is blocked for this

@Romfos, note that the .NET 10 SDK can be installed before the AppVeyor CI run commences: 4f624b0. (Relevant AppVeyor documentation for that is here.) The tradeoff being that the AppVeyor builds become even slower.

(To be honest, I'm getting around to your point of view: it'll be good to be rid of it ASAP. That being said, see my comment in #699 (comment). Let's probably do this for the next release after 6.0.0.)

@stakx stakx changed the title Drop appveyor in favor of github actions Drop AppVeyor in favor of GitHub Actions Dec 10, 2025
@Romfos
Copy link
Contributor Author

Romfos commented Dec 11, 2025

ok, rebased

cc @jonorossi

@stakx
Copy link
Member

stakx commented Dec 12, 2025

FYI, I've also contacted jonorossi regarding the NuGet API key in GitHub Actions config. Let's proceed once that is set up.

@stakx stakx marked this pull request as draft December 17, 2025 18:04
@stakx
Copy link
Member

stakx commented Dec 17, 2025

@Romfos, I'm marking this PR as a "draft" to prevent accidental merging. I want to make sure the NuGet publishing on GitHub Actions is fully set up before this lands on master. But I'm still on board with this!

@304NotModified
Copy link

304NotModified commented Dec 26, 2025

Very nice that build.cmd/build.sh is not needed anymore 👍

Maybe also update the readme (build.cmd and build.sh is still there)

See:
https://github.com/search?q=repo%3Acastleproject%2FCore%20build.cmd&type=code

@Romfos
Copy link
Contributor Author

Romfos commented Dec 29, 2025

Very nice that build.cmd/build.sh is not needed anymore 👍

Maybe also update the readme (build.cmd and build.sh is still there)

See: https://github.com/search?q=repo%3Acastleproject%2FCore%20build.cmd&type=code

updated. branch is rebased

@stakx
Copy link
Member

stakx commented Dec 30, 2025

@Romfos, this PR goes quite a bit beyond giving up AppVeyor. From a quick glance over it, it also appears to be doing these things:

  1. Test projects setup

    • marks test projects as non-packable
    • makes the tests in the Castle.Core.Tests.WeakNamed.csproj project runnable again (by adding the test framework & adapter packages to the .csproj file)
    • replaces NUnitLite with NUnit
  2. Build scripts simplification

    • replaces all those build scripts with simple invocations of dotnet build / dotnet test
  3. Assembly output

    • sets up SourceLink
    • sets up deterministic builds

I suspect that many (perhaps even all) of these additional changes could be merged now, without us waiting for the switch away from AppVeyor. If you would like to submit these changes as one or several focused PRs, I'd be willing to review and merge them for the upcoming 6.0.0 release.

@Romfos
Copy link
Contributor Author

Romfos commented Dec 30, 2025

@stakx

Technically this is possible, but I'm think that probably would be better to merge this as is, rather then make modifications for AppVeyor...

Maybe we can just merge it?
You can get nuget packages without having NUGET_API_KEY, because publishing to nuget is optional in release build (see screenshot with checkbox).

You can run release build, get nuget packages from build attachments, and publish them manually to nuget if needed
NUGET_API_KEY could be added later. This is not a blocker at all.

@stakx
Copy link
Member

stakx commented Dec 30, 2025

Maybe we can just merge it?

Unfortunately, we cannot, because once we do, we can no longer publish NuGet packages as the necessary API key hasn't been configured yet on GitHub Actions, and I do not want to risk upcoming releases being unnecessarily held back by this.

You can get nuget packages without having NUGET_API_KEY, because publishing to nuget is optional in release build (see screenshot with checkbox).

What is more important: getting this PR merged as quickly as possible, or remaining able to publish new releases to the location where devs are most likely to look for them and pull them from? I think it's clearly the latter. Having to get a newly released package version from a location other than NuGet (say, from GitHub) may be OK for pre-releases, but IMHO it won't do for regular releases.

[...] publish them manually to nuget if needed [...]

I personally don't have the necessary permissions for that.

@Romfos
Copy link
Contributor Author

Romfos commented Dec 30, 2025

Maybe we can just merge it?

Unfortunately, we cannot, because once we do, we can no longer publish NuGet packages as the necessary API key hasn't been configured yet on GitHub Actions, and I do not want to risk upcoming releases being unnecessarily held back by this.

You can get nuget packages without having NUGET_API_KEY, because publishing to nuget is optional in release build (see screenshot with checkbox).

What is more important: getting this PR merged as quickly as possible, or remaining able to publish new releases to the location where devs are most likely to look for them and pull them from? I think it's clearly the latter. Having to get a newly released package version from a location other than NuGet (say, from GitHub) may be OK for pre-releases, but IMHO it won't do for regular releases.

[...] publish them manually to nuget if needed [...]

I personally don't have the necessary permissions for that.

fine. I'm not so familiar with appveyor syntax, you can do if you know how to implement it in a right way

@stakx
Copy link
Member

stakx commented Dec 31, 2025

I'm not so familiar with appveyor syntax, you can do if you know how to implement it in a right way

I meant precisely that quite a few of your proposed changes don't seem related to AppVeyor at all, and could be made anytime, without touching the CI configuration.

If I did get around to implementing some of your proposed changes before you get to it, would you mind if I'd cherry-pick (and adapt) your commit, so that you get attributed for your work in the commit history?

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