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

Fixing documentation checks and updating Travis build #1335

Merged
merged 23 commits into from
Jul 29, 2018

Conversation

rolanddenis
Copy link
Member

@rolanddenis rolanddenis commented Jul 11, 2018

PR Description

Fixing documentation checks that was not fully done (see #1334)

Checklist

  • Doxygen documentation of the code completed (classes, methods, types, members...)
  • New entry in the ChangeLog.md added.
  • All continuous integration tests pass (Travis & appveyor)
  • Fixing all doxygen warnings
  • [ ] Adding @example tag check

@rolanddenis rolanddenis changed the title [WIP] Fixing document checks [WIP] Fixing documentation checks Jul 11, 2018
@rolanddenis rolanddenis changed the title [WIP] Fixing documentation checks Fixing documentation checks and updating Travis build Jul 12, 2018
Copy link
Member

@dcoeurjo dcoeurjo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks @rolanddenis for this PR ;)
I just have a minor comment. Could you please also add an entry in the Changelog?

cd "$BUILD_DIR"
make exampleDiscreteExteriorCalculusChladni
#make exampleDiscreteExteriorCalculusSolve
#make exampleDECSurface
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why these one have been commented ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a copy-paste from .travis/main_build.sh:

#make exampleDiscreteExteriorCalculusSolve;

and it has been commented in 163a835 from #1306 but I don't know why (@kerautret ?).

I will try to uncomment these lines :octocat:

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was to try to avoid the segfault or timeout issue of travis due to gcc problem (as discussed with Jaco in last meeting). Changing the moment where it was compiled fixed the memory issue (I suppose), but perhaps we have to remove this example from the travis test.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, these two examples are already built during the main build. This part is only for examples that need to be build alone in order to avoid compiler failures. I think we can remove these two lines ...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oups, didn't see your answer 😉

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes of course if they are in the main part, but it was commented because it was an alternative.
But this part is not robust in all cases.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering ;)

@dcoeurjo dcoeurjo added this to Inbox in Issue triage via automation Jul 28, 2018
@dcoeurjo dcoeurjo added this to the 1.0 milestone Jul 28, 2018
@rolanddenis
Copy link
Member Author

About the ChangeLog.md, I've already add an entry (see https://github.com/DGtal-team/DGtal/pull/1335/files#diff-e705c8fadf1193ab59443a5e6c8cbe8b) 😉

@dcoeurjo
Copy link
Member

Oops.. sorry ;)
Thanks for the PR

Just a quick question where do the SCRIPTBEGIN/END come from ?

@dcoeurjo dcoeurjo merged commit f879fc4 into DGtal-team:master Jul 29, 2018
Issue triage automation moved this from Inbox to Done Jul 29, 2018
@rolanddenis
Copy link
Member Author

They are defined in .travis/before_global.sh:

export SCRIPT_BEGIN="set -$SET_OPTIONS"

The idea is to set and reset some flags useful for debugging Travis, like the set -e that was already set in some scripts before this PR and that make a script returning a non-zero value if any of its command fails. We can also add v to view each executed commands. The SCRIPT_END resets the flags (because some scripts always have errors, like .travis/install_deps_macos.sh and Travis also have a strange behaviour when the e flag is set) and resets the working directory.

@rolanddenis
Copy link
Member Author

rolanddenis commented Jul 29, 2018

But it seems that it doesn't always work 😕 : the Travis job associated to the merge has successful ended (see https://travis-ci.org/DGtal-team/DGtal/builds/404334020 ) but the .travis/publish_doc.sh has partially failed (see the end of https://api.travis-ci.org/v3/job/409458892/log.txt ). So the documentation is up to date, but not the tags and docset ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Issue triage
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants