-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Build tutorials on Azure Pipelines #2696
Build tutorials on Azure Pipelines #2696
Conversation
903bb88
to
3883120
Compare
OK, not as bad as I expected, 61 out of 72 tutorials can still be built!
Unfortunately, this adds another 25+ minutes to our Ubuntu pipeline. Question is, do we really need this to run on every pull request or merge? AFAIU Azure supports pipelines that are inactive by default, but can be triggered manually or periodically. Perhaps we can opt for this. |
Great job.
Agree with both modalities. There are some PRs where we clearly know it will affect downstream targets so the manual option is important here. The periodic check (once a week?) will also grant us some level of reassurance. |
dcbb7f8
to
2d10543
Compare
I have dived into Azure documentation and came up with 4 ways to incorporate tutorial builds into CI.
1 is long. 2a is also long, plus manual triggering for pull requests is awkward, you need to copy commit ID. 2b seemed to be the best. The time to complete the checks is not increased, but if we have reasons to believe that PR may affect tutorials, we wait additional 30 minutes until that extra pipeline is completed. Unfortunately, turned out to be impossible, because "build triggers" don't work for PR. So it would only be possible to run this additional pipeline on merges to master, which is too late to catch problems. 2c is like 1, long and even occupies more resources. However, I had an idea that we can set Library is done in 25 minutes, plus tutorials take another 40. So in the end the pipeline a bit over 1 hour, which means it's actually faster than the rest of "Build.xxx" pipelines. Seems like the best approach to me now. The only downside is that we are occupying one more build agent. |
Thank you for the detailed breakdown. I would say at this point is hard to make a decisions based on the inconveniences having one more build agent, so I would kick in with option 2c as you suggested, especially because everything else seems to come with worse inconveniences. |
OK. One other thing that I noticed is that not all of the existing tutorials are included in the TOC. For example, "Removing outliers using a ConditionalRemoval filter" is not on the tutorials webpage. We need to go through this at some point and either include in the list or delete such dangling tutorials. As for this PR, the remaining work items are:
I'm actually not sure anymore if this should close #2672. In it's current state this pipeline only builds tutorials on Ubuntu, so we can not be sure that downstream projects can configure/build against PCL on other platforms. |
We can always have an extra step like you did in 1 where we build just a single target, filled with one class from each module. We compile and run the executable to ensure things don't segfault magically. |
43e6fe7
to
5800d5f
Compare
This switch is already on in Windows builds. This submodule is needed to build the "bspline_fitting" tutorial.
The cluster limits are the same as in: ba2f4ca
5800d5f
to
6e16e17
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this still a WIP? Looks pretty finished. There's some lines which are not following the style guide, but I'm assuming you did that to keep consistency with the surrounding code.
Yep, this is ready now. The style is left as-is for consistency. In my latest commit I enabled |
Should we add some fancy badges for the docs and tutorials on the README.md? |
If you want... :) |
This PR adds a new Ubuntu-based pipeline "Tutorials" which builds and installs PCL and then tries to build all tutorials as standalone projects. The pipeline will fail in case of configuration or compilation errors, including warnings. (The latter is to catch deprecations.)
Additionally several currently broken tutorials are fixed.
Edit:
Edit: Closes 2672(not anymore).