cvut / qtrvsim

RISC-V CPU simulator for education purposes
GNU General Public License v3.0
466 stars 56 forks source link

CI fix #98

Closed ArielHeleneto closed 6 months ago

ArielHeleneto commented 7 months ago
jdupak commented 7 months ago

Hi Ariel, thank you for your contribution and welcome.

However, I am unsure what these changes aim to achieve. The only purpose of this build is to run automatic testing. It seems unnecessary to build a release version for each PUSH/MERGE. The release version only differs in compiler optimizations, reduced logging, and the absence of sanitizers. The files are not meant for distribution, release artifacts are built manually. Could you clarify your intentions?

As for the windeployqt - I am not opposed if that helps Windows developers. Still, I am unsure, why would you want to run the created artifacts locally.

ArielHeleneto commented 7 months ago

Hi Ariel, thank you for your contribution and welcome.

However, I am unsure what these changes aim to achieve. The only purpose of this build is to run automatic testing. It seems unnecessary to build a release version for each PUSH/MERGE. The release version only differs in compiler optimizations, reduced logging, and the absence of sanitizers. The files are not meant for distribution, release artifacts are built manually. Could you clarify your intentions?

As for the windeployqt - I am not opposed if that helps Windows developers. Still, I am unsure, why would you want to run the created artifacts locally.

It would be easy for someone to test a CI version when they want to try new features or bug fixes. For Windows, people can't use it without dll being packed. Thus I add windeployqt to pack dll automatically.

It is unnecessary to build a release version for each PUSH/MERGE. The release version only differs in compiler optimizations, reduced logging, and the absence of sanitizers. But you need to test release build while someone want to try them before we publish one.

Not everyone want to run our program through compile our code since it's difficult to prepare an enviourment in Windows without a guide.

jdupak commented 6 months ago

Sorry for the silent time @ArielHeleneto, I was busy.

I have committed the part with winqtdeploy (with you as the author), I like that part. As for the release builds, I don't want to have it here (debug.yml which is supposed to run for every push), so I am closing this issue.

What would be interesting for me would be to abstract out the build into separate action and use it both from the debug.yml and release.yml. We can have something like you suggested, but it would need to be triggered manually. Feel free to look at that and ask any questions, if you feel like it (probably in a new issue).