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

Try enabling PR build triggers #113

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Numpsy
Copy link
Contributor

@Numpsy Numpsy commented Feb 22, 2024

I think it would be useful to have the CI build linked to pull requests to catch any problems with tests more easily - but I haven't much experience with setting that up between Azure and Github.

Opening this to raise the question though.

@ironfede
Copy link
Owner

I haven't a lot of experience on pros and cons for a pr trigger. I would prefer to have some opinion on this point before merging.
Thank you for the point @Numpsy

@Numpsy
Copy link
Contributor Author

Numpsy commented Feb 25, 2024

I use Azure builds at work and have worked on CI builds there, and have worked on other GitHub projects that use GitHub actions, but haven't really done anything with the integration between the two, so I'm not sure what the best approach for it is myself.

@jeremy-visionaid
Copy link
Contributor

@ironfede @Numpsy I'm more of a Gerrit + Jenkins kind of guy, but CI on PRs would be awesome. I don't mind spending some time figuring this one out too if you want any help with it.

@jeremy-visionaid
Copy link
Contributor

I've set up a request to enable Azure Pipelines for the Visionaid International fork, so hopefully I'll get to play around with it there
https://github.com/Visionaid-International-Ltd/openmcdf

@jeremy-visionaid
Copy link
Contributor

Well, I got an email to say that my request for free tier parallel builds was completed, but I still get an error when trying to run a job:
No hosted parallelism has been purchased or granted.

Not a great surprise to me given my previous dealings with Azure, pretty much everything seems to be flaky during setup. Maybe it will just start working randomly later in the week.

FWIW, the change to trigger on pull requests looks correct to me. However, I think using a Windows image would be better, since the whole solution could be built, including multi-targeting. i.e. it would be both simpler and give better coverage.

@Numpsy
Copy link
Contributor Author

Numpsy commented Sep 29, 2024

Maybe I asked this before and forgot the answer, but is there particularly a need to run the CI builds in Azure rather than Github actions these days?

I'm not sure if having the builds over here could make it easier to plug in things like CodeQL later as well if there is a benefit in that.

@jeremy-visionaid
Copy link
Contributor

@Numpsy I agree, GitHub Actions seems like it might be a bit simpler. Seems that MS processed my request incorrectly for Azure Piplelines, even though I filled out the forms correctly, so I can't use/evaluate it. I've resubmitted my requests, but who know if/when they'll fix it.

@ironfede
Copy link
Owner

ironfede commented Oct 4, 2024

I've added a github actions workflow for build and test. It's only a basic one so I think that it can be enhanced in the future but it should already have pr trigger enabled @Numpsy and @jeremy-visionaid .
Please let me know if it's working correctly.

@Numpsy
Copy link
Contributor Author

Numpsy commented Oct 5, 2024

Is seems to be working OK.

Id thought I might have a go at plugging https://github.com/Tyrrrz/GitHubActionsTestLogger into the build to get test results published, but it doesn't look like it'll support .NET 4.5 test projects (requires .NET 4.6.2), so it'd need to either be conditionalised or left till later

@Numpsy
Copy link
Contributor Author

Numpsy commented Oct 5, 2024

Actually, I wonder if it should have /p:ContinuousIntegrationBuild=true added to the dotnet build call in the yaml file -

It currently has

  <PropertyGroup Condition="'$(TF_BUILD)' == 'true'">
    <ContinuousIntegrationBuild>true</ContinuousIntegrationBuild>
  </PropertyGroup>

in the projects and I don't think TF_BUILD will be set in Github?
I think it'd be easier to set the property in the CI build script rather than in the project though

@ironfede
Copy link
Owner

ironfede commented Oct 5, 2024

Actually, I wonder if it should have /p:ContinuousIntegrationBuild=true added to the dotnet build call in the yaml file -

It currently has

  <PropertyGroup Condition="'$(TF_BUILD)' == 'true'">
    <ContinuousIntegrationBuild>true</ContinuousIntegrationBuild>
  </PropertyGroup>

in the projects and I don't think TF_BUILD will be set in Github? I think it'd be easier to set the property in the CI build script rather than in the project though

Thanks! I'll take a look asap!

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