Skip to content
This repository has been archived by the owner on Sep 27, 2021. It is now read-only.

Piece wise beadmotifconnector #475

Merged
merged 17 commits into from
Dec 20, 2019
Merged

Conversation

JoshuaSBrown
Copy link
Contributor

Used pragma once
Const correctness applied
Improved testing
noexcept keyword added where appropriate
assertion test added in appropriate method

@codecov
Copy link

codecov bot commented Dec 13, 2019

Codecov Report

Merging #475 into master will increase coverage by <.1%.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #475     +/-   ##
========================================
+ Coverage    61.1%   61.1%   +<.1%     
========================================
  Files         129     129             
  Lines        6321    6322      +1     
========================================
+ Hits         3868    3869      +1     
  Misses       2453    2453
Flag Coverage Δ
#gcc 61.1% <100%> (ø) ⬆️
Impacted Files Coverage Δ
include/votca/csg/beadmotifconnector.h 100% <ø> (ø) ⬆️
src/libcsg/beadmotifconnector.cc 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 57b6fea...25bf815. Read the comment docs.

@JoshuaSBrown
Copy link
Contributor Author

@JensWehner hey can you take a look when you get the chance?

@@ -15,21 +15,22 @@
*
*/

#include <votca/csg/beadmotifconnector.h>
#include "../../include/votca/csg/beadmotifconnector.h"
Copy link
Member

Choose a reason for hiding this comment

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

Why?

@JoshuaSBrown
Copy link
Contributor Author

I believe it is a better practice. For instance if you build the project and it has previously installed it can link to the wrong files.

@JoshuaSBrown
Copy link
Contributor Author

It is safer to reserve global reference for the other repositories, and use the relative include paths internally, it doesn't look as nice though.

@junghans
Copy link
Member

I think it doesn't hurt, we add the local include path here anyhow:
https://github.com/votca/csg/blob/master/src/libcsg/CMakeLists.txt#L32

@JoshuaSBrown
Copy link
Contributor Author

My wording is probably a bit off, <> notation looks somewhere on the system, whereas "" will look in the current repo.

@JoshuaSBrown
Copy link
Contributor Author

JoshuaSBrown commented Dec 15, 2019

I have had issues with this in the past but I am not sure if they were before or after those lines were placed in the cmake file.

@junghans
Copy link
Member

@JensWehner please review

@JoshuaSBrown
Copy link
Contributor Author

Ping @JensWehner

@junghans junghans merged commit 4516c92 into master Dec 20, 2019
@junghans junghans deleted the piece-wise-beadmotifconnector branch December 20, 2019 03:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants