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

[Jenkins] New warnings publisher syntax #2206

Merged
merged 12 commits into from Sep 18, 2018

Conversation

Projects
None yet
3 participants
@bilke
Copy link
Member

bilke commented Sep 13, 2018

See https://jenkins.io/blog/2018/09/11/speaker-blog-warnings-plugin/

Provides a better overview over warnings. I suggest to

  • Fix warnings
  • Exclude remaining warnings
  • Set warning thresholds (to 0 or a specified number) to mark build as unstable
@endJunction

This comment has been minimized.

Copy link
Member

endJunction commented Sep 13, 2018

All of the doxygen warnings are ignorable. There two types max_graph_nodes too small, and a warning in functional.h about recursion.

@bilke

This comment has been minimized.

Copy link
Member Author

bilke commented Sep 13, 2018

It seems that filtering by warning message is not possible yet: https://github.com/jenkinsci/warnings-plugin/blob/master/doc/Documentation.md#filtering-issues

Before we filtered the Doxygen warnings. (.*DOT_GRAPH_MAX_NODES. and *potential recursive class relation.*)

@endJunction endJunction force-pushed the bilke:jenkins-warnings branch from 92d103a to 4b60808 Sep 14, 2018

@endJunction

This comment has been minimized.

Copy link
Member

endJunction commented Sep 14, 2018

One of the MSVC warnings ('void (__cdecl *volatile )(void)': top-level volatile in cast is ignored) could be potentially fixed by

diff --git a/Applications/CLI/ogs_embedded_python.cpp b/Applications/CLI/ogs_embedded_python.cpp
index 2da7a23d1..39b6e3e28 100644
--- a/Applications/CLI/ogs_embedded_python.cpp
+++ b/Applications/CLI/ogs_embedded_python.cpp
@@ -28,7 +28,7 @@ PYBIND11_EMBEDDED_MODULE(OpenGeoSys, m)
 template <typename T>
 void mark_used(T p)
 {
-    volatile T vp = (volatile T)p;
+    volatile T vp = p;
     vp = vp;
 }

but since I'm not able to use shared libraries with python, I cannot test its correctness.

@endJunction endJunction force-pushed the bilke:jenkins-warnings branch from 08da809 to 83c98bb Sep 14, 2018

@endJunction

This comment has been minimized.

Copy link
Member

endJunction commented Sep 14, 2018

Another deprecated warning is the QVTKWidget, which is to be replaced with QVTKOpenGLWidget.
Starting naïve commit produces a segv on DataExplorer's start because the get render window returns null in visualizationWidget ctor

vtkWidget->GetRenderWindow()->GetInteractor()->SetInteractorStyle(_interactorStyle);
``

@endJunction endJunction force-pushed the bilke:jenkins-warnings branch from 83c98bb to 4ec3f5d Sep 15, 2018

@chleh

This comment has been minimized.

Copy link
Collaborator

chleh commented Sep 17, 2018

One of the MSVC warnings ('void (__cdecl *volatile )(void)': top-level volatile in cast is ignored) could be potentially fixed by

diff --git a/Applications/CLI/ogs_embedded_python.cpp b/Applications/CLI/ogs_embedded_python.cpp
index 2da7a23d1..39b6e3e28 100644
--- a/Applications/CLI/ogs_embedded_python.cpp
+++ b/Applications/CLI/ogs_embedded_python.cpp
@@ -28,7 +28,7 @@ PYBIND11_EMBEDDED_MODULE(OpenGeoSys, m)
 template <typename T>
 void mark_used(T p)
 {
-    volatile T vp = (volatile T)p;
+    volatile T vp = p;
     vp = vp;
 }

but since I'm not able to use shared libraries with python, I cannot test its correctness.

@endJunction, that code is intended to capture static libs + Python. Is there a problem for you using shared libs and Python?

@endJunction

This comment has been minimized.

Copy link
Member

endJunction commented Sep 17, 2018

@chleh Yes. See also #2184 (comment) . Let's continue the discussion there. I'll be in the office after lunch.

@endJunction endJunction force-pushed the bilke:jenkins-warnings branch from 4ec3f5d to 912c4b7 Sep 18, 2018

@bilke bilke force-pushed the bilke:jenkins-warnings branch from 9bf003e to df657c0 Sep 18, 2018

@bilke bilke force-pushed the bilke:jenkins-warnings branch from df657c0 to 2518453 Sep 18, 2018

@bilke

This comment has been minimized.

Copy link
Member Author

bilke commented Sep 18, 2018

@endJunction Thanks for your fixes!

@bilke bilke merged commit a949b3c into ufz:master Sep 18, 2018

2 of 3 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
deploy/netlify Deploy preview ready!
Details

@bilke bilke deleted the bilke:jenkins-warnings branch Sep 18, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.