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

compilation fixes for clang-10s more strict checking #11316

Merged

Conversation

kvahed
Copy link
Contributor

@kvahed kvahed commented Mar 20, 2020

Pull Request Guidelines

Pull requests are an essential collaborative tool for modern software development.

The below list is intended to help you figure out whether your code is ready to be reviewed
and merged into ArangoDB. The overarching goal is to:

  • Reduce the amount of recurring defects
  • Reduce the impact to the other developer’s time and energy spent on defects in your code
  • Increase the overall autonomy and productivity of individual developers

Acceptance Checklist

The below list is not exhaustive, think thoroughly whether the provided information is sufficient.
Remove options that do not apply

Scope & Purpose

(Can you describe what functional change your PR is trying to effect?)

  • Bug-Fix for devel-branch (i.e. no need for backports?)
  • Bug-Fix for a released version (did you remember to port this to all relevant release branches?)
  • Strictly new functionality (i.e. a new feature / new option, no need for porting)
  • The behavior in this PR can be (and was) manually tested (support / qa / customers can test it)
  • The behavior change can be verified via automatic tests

Related Information

(Please reference tickets / specification etc )

  • There is a GitHub Issue reported by a Community User:
  • There is an internal planning ticket:
  • There is a JIRA Ticket number (In case a customer was affected / involved):
  • There is a design document:

Testing & Verification

(Please pick either of the following options)

This change is a trivial rework / code cleanup without any test coverage.

(or)

This change is already covered by existing tests, such as (please describe tests).

(or)

This PR adds tests that were used to verify all changes:

  • Added Regression Tests (Only for bug-fixes)
  • Added new C++ Unit Tests (Either GoogleTest or Catch-Test)
    • Did you add tests for a new RestHandler subclass ?
    • Did you add new mocks of underlying code layers to be able to verify your functionality ?
    • ...
  • Added new integration tests (i.e. in shell_server / shell_server_aql)
  • Added new resilience tests (only if the feature is impacted by failovers)

Additionally:

  • There are tests in an external testing repository (i.e. node-resilience tests, chaos tests)
  • I ensured this code runs with ASan / TSan or other static verification tools

(Include link to Jenkins run etc)

Think about whether the new code you added is modular enough to be
easily testable by unit tests written with GTest / Catch. It is not good if your feature is so interconnected
that it prevents other people from writing their own unit gests. It should be possible
to use your code in future without extensively mocking your classes.
A bad example that required some extensive effort would be the storage engine API.

Documentation

All new Features should be accompanied by corresponding documentation.
Bugs and features should furthermore be documented in the changelog so that
developers and users have a concise overview.

  • Added a Changelog Entry (referencing the corresponding public or internal issue number)
  • Added entry to Release Notes
  • Added a new section in the Manual
  • Added a new section in the http API
  • Added Swagger examples for the http API

CLA Note

Please note that for legal reasons we require you to sign the Contributor Agreement
before we can accept your pull requests.

@kvahed kvahed added the 1 Bug label Mar 20, 2020
@kvahed kvahed requested a review from jsteemann March 20, 2020 11:56
@kvahed kvahed self-assigned this Mar 20, 2020
@kvahed
Copy link
Contributor Author

kvahed commented Mar 20, 2020

@jsteemann
Copy link
Contributor

jsteemann commented Mar 20, 2020

@kvahed
Copy link
Contributor Author

kvahed commented Mar 20, 2020

image

@kvahed
Copy link
Contributor Author

kvahed commented Mar 20, 2020

image

kvahed added 2 commits March 20, 2020 22:02
…-fix/unnecessary-copying-and-wrong-mutex-declarations
@kvahed
Copy link
Contributor Author

kvahed commented Mar 20, 2020

devel merged in.

@jsteemann jsteemann merged commit e4d5a5d into devel Mar 21, 2020
@jsteemann jsteemann deleted the bug-fix/unnecessary-copying-and-wrong-mutex-declarations branch March 21, 2020 10:55
ObiWahn added a commit that referenced this pull request Mar 25, 2020
…telisting

* origin/devel: (80 commits)
  Feature/aql subquery execution block impl execute implementation batch sub queries (#11318)
  Fix isAdminUser. (#11326)
  Bug fix/fixes 20200318 (#11319)
  updated CHANGELOG
  Feature/out of search in range (#11324)
  fix "fix" for collection figures (#11323)
  updated CHANGELOG
  compilation fixes for clang-10s more strict checking (#11316)
  fix failing query (#11317)
  KShortestPathsExecutor must reset its KShortestPathFinder, including all caches. (#11312)
  Feature/aql subquery execution block impl execute implementation expected number of rows (#11274)
  Add DTRACE points to measure request timings. (#11245)
  USE_STRICT_OPENSSL is Off by default
  Fix usesRevisionAsDocumentId population and add syncByRevision flag (#11314)
  Traversal Bugfix  (#11310)
  Bug fix/issue 11275 (#11299)
  added simple test (#11301)
  Fix some typos (#10173)
  Documentation/typos 2020-01-24 (#10975)
  Update CHANGELOG
  ...
ObiWahn added a commit that referenced this pull request Mar 27, 2020
…ql-functions

* origin/devel: (25 commits)
  Bug fix/fixes 20200318 (#11319)
  updated CHANGELOG
  Feature/out of search in range (#11324)
  fix "fix" for collection figures (#11323)
  updated CHANGELOG
  compilation fixes for clang-10s more strict checking (#11316)
  fix failing query (#11317)
  KShortestPathsExecutor must reset its KShortestPathFinder, including all caches. (#11312)
  Feature/aql subquery execution block impl execute implementation expected number of rows (#11274)
  Add DTRACE points to measure request timings. (#11245)
  USE_STRICT_OPENSSL is Off by default
  Fix usesRevisionAsDocumentId population and add syncByRevision flag (#11314)
  Traversal Bugfix  (#11310)
  Bug fix/issue 11275 (#11299)
  added simple test (#11301)
  Fix some typos (#10173)
  Documentation/typos 2020-01-24 (#10975)
  Update CHANGELOG
  Some cleanup for new executor test code, which accidentally fixes ASAN failures in ExecutorTestHelper (#11283)
  LZ4 update (#11306)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants