Skip to content
Snippets Groups Projects

Check sequences for consistency: forbid identical sequences at different depths…

Merged Stewart Martin-Haugh requested to merge smh/athena:check_subsequences into master
All threads resolved!

Check sequences for consistency: forbid identical sequences at different depths and add unit tests to check.

Improved version of !20390 (closed)

Fix for ATEAM-501

Edited by Stewart Martin-Haugh

Merge request reports

Pipeline #680516 passed

Pipeline passed for 5dd72288 on smh:check_subsequences

Approval is optional

Merged by Frank WinklmeierFrank Winklmeier 6 years ago (Jan 30, 2019 3:50pm UTC)

Merge details

  • Changes merged into master with 6999695d (commits were squashed).
  • Deleted the source branch.

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Tomasz Bold approved this merge request

    approved this merge request

  • added 1 commit

    Compare with previous version

  • This merge request affects 1 package:

    • Control/AthenaConfiguration

    Adding @ssnyder as watcher

  • Stewart Martin-Haugh resolved all discussions

    resolved all discussions

  • :white_check_mark: CI Result SUCCESS

    Athena AthSimulation
    externals :white_check_mark: :white_check_mark:
    cmake :white_check_mark: :white_check_mark:
    make :white_check_mark: :white_check_mark:
    required tests :white_check_mark: :white_check_mark:
    optional tests :cloud: :white_check_mark:

    Full details available at NICOS MR-20444-2019-01-21-21-33
    :white_check_mark: Athena: number of compilation errors 0, warnings 0
    :white_check_mark: AthSimulation: number of compilation errors 0, warnings 0
    For experts only: Jenkins output [CI-MERGE-REQUEST 32484] (for remote access see the FAQ for MR reviewers)

  • There are two comments that seem to be obsolete. It looks OK but it would be nice if the comments can be removed.

    Thanks, Tadej (L1)

  • Hi Stewart, if you are going to address the comments from Tadej you may also consider grouping sequences manipulation in here: https://gitlab.cern.ch/atlas/athena/blob/master/Control/AthenaCommon/python/CFElements.py

  • added 1 commit

    • 8c081c97 - Move sequence checks to CFElements

    Compare with previous version

  • Stewart Martin-Haugh resolved all discussions

    resolved all discussions

  • Hi @tbold,

    I moved the functions over, but not the unit tests. I think it could be improved further by overriding the + operator for AlgSequence but I'm not entirely clear how to do that.

    Cheers,

    Stewart

  • Jenkins please retry a build

  • This merge request affects 2 packages:

    • Control/AthenaCommon
    • Control/AthenaConfiguration

    Adding @ssnyder ,@rbianchi as watchers

  • :negative_squared_cross_mark: CI Result FAILURE

    Athena AthSimulation
    externals :white_check_mark: :white_check_mark:
    cmake :white_check_mark: :white_check_mark:
    make :white_check_mark: :white_check_mark:
    required tests :o: :o:
    optional tests :cloud: :white_check_mark:

    Full details available at NICOS MR-20444-2019-01-29-02-56
    :warning: Athena: number of compilation errors 0, warnings 1
    :white_check_mark: AthSimulation: number of compilation errors 0, warnings 0
    :pencil: CI Jenkins server is switched to https://atlas-sit-ci.cern.ch. It is accessible world-wide (behind CERN SSO). In old links to Jenkins server aibuild080.cern.ch:8080 should be replaced with atlas-sit-ci.cern.ch For experts only: Jenkins output [CI-MERGE-REQUEST 32905]

  • Hi @smh, python linting is failing for CFElements.py

    /var/lib/jenkins/workspace/CI-MERGE-REQUEST/master/Control/AthenaCommon/python/CFElements.py:32:-95: W605 invalid escape sequence '\_'
    /var/lib/jenkins/workspace/CI-MERGE-REQUEST/master/Control/AthenaCommon/python/CFElements.py:32:-59: W605 invalid escape sequence '\_'
    /var/lib/jenkins/workspace/CI-MERGE-REQUEST/master/Control/AthenaCommon/python/CFElements.py:32:-25: W605 invalid escape sequence '\_'
  • added 1 commit

    • 5dd72288 - Fix backslash interpreted as escape character

    Compare with previous version

  • This merge request affects 2 packages:

    • Control/AthenaCommon
    • Control/AthenaConfiguration

    Adding @ssnyder ,@rbianchi as watchers

  • :white_check_mark: CI Result SUCCESS

    Athena AthSimulation
    externals :white_check_mark: :white_check_mark:
    cmake :white_check_mark: :white_check_mark:
    make :white_check_mark: :white_check_mark:
    required tests :white_check_mark: :white_check_mark:
    optional tests :cloud: :white_check_mark:

    Full details available at NICOS MR-20444-2019-01-29-18-09
    :warning: Athena: number of compilation errors 0, warnings 1
    :white_check_mark: AthSimulation: number of compilation errors 0, warnings 0
    :pencil: CI Jenkins server is switched to https://atlas-sit-ci.cern.ch. It is accessible world-wide (behind CERN SSO). In old links to Jenkins server aibuild080.cern.ch:8080 should be replaced with atlas-sit-ci.cern.ch For experts only: Jenkins output [CI-MERGE-REQUEST 32947]

  • CI looks fine now and changes are also ok from my point of view. If there are no further comments from @tadej or @tbold, MR can be approved.

    Kira (L1)

  • All looks fine to me.

  • Approving then.

    Kira (L1)

  • added review-approved label and removed review-pending-level-1 label

  • mentioned in commit 6999695d

  • Please register or sign in to reply
    Loading