notepad-plus-plus / nppShell

Provide Explorer context menu entry "Edit with Notepad++"
GNU General Public License v3.0
25 stars 13 forks source link

added github CI action and appveyor #3

Closed chcg closed 1 year ago

chcg commented 1 year ago

Is it intended that the win32 build doesn't create a msix package?

donho commented 1 year ago

@chcg

Is it intended that the win32 build doesn't create a msix package?

Yes. Win32 build doesn't need MSIX package.

donho commented 1 year ago

@chcg I thought .github/workflows/CI_build.yml can replace appveyor.yml. Do we really need both systems?

GurliGebis commented 1 year ago

I would suggest only using github actions if possible

chcg commented 1 year ago

Updated PR:

donho commented 1 year ago

@chcg

  • no artifact upload for Win32

Does it mean x32 is not compiled by CI? If the answer is no, why can't we have Win32 to be compiled/verified? Otherwise never mind - once we have got all platforms are compiled/verified, it doesn't matter with or without artifact. Because the dll needs to be signed.

GurliGebis commented 1 year ago

I agree, skip creating artifacts and instead just build all three architectures

chcg commented 1 year ago

See https://github.com/chcg/nppShell/actions/runs/4536495216. Builds are done in Debug and Release for x64, Win32, arm64. Artifacts upload (aka storage as zip) is just an additional step after the build itself and currently done for the msi packages.

GurliGebis commented 1 year ago

@donho and @chcg I would to enable Treat warnings as errors - how do you feel about that?

GurliGebis commented 1 year ago

From a continuous integration point of view, it would be better to make sure the build fails on warnings, since they should not be ignored.

GurliGebis commented 1 year ago

I have created a PR that enables this 🙂

GurliGebis commented 1 year ago

@donho shouldn't we enable github actions to do CI on this repo?

donho commented 1 year ago

@GurliGebis Yes, we should. I didn't close it, and I cannot reopen it - I guess it's closed while I removed main branch (The repo is switched to master from main).

@chcg Could you please redo the PR on master?

GurliGebis commented 1 year ago

That makes sense 🙂