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

Add make check target #4002

Merged
merged 7 commits into from
Dec 20, 2018
Merged

Add make check target #4002

merged 7 commits into from
Dec 20, 2018

Conversation

driusan
Copy link
Collaborator

@driusan driusan commented Oct 15, 2018

This adds a "make check" target to perform basic unittests and
does some minor refactoring of the makefile

The target "make checkstatic" and "make unittests" are also
added to make it easier to quickly run a smoke test.

Copy link
Contributor

@PapillonMcGill PapillonMcGill left a comment

Choose a reason for hiding this comment

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

need to fix db access for the unittest

Copy link
Contributor

@PapillonMcGill PapillonMcGill left a comment

Choose a reason for hiding this comment

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

just so it will remove my approval as Travis complain

This adds a "make check" target to perform basic unittests and
does some minor refactoring of the makefile

The target "make checkstatic" and "make unittests" are also
added to make it easier to quickly run a smoke test.
@driusan driusan added Meta PR does something that organizes, upgrades, or manages the functionality of the codebase "Help! I don't understand Travis!" PR is having a beef with TRAVIS. Someone needs to help [branch] major labels Dec 19, 2018
driusan added a commit to driusan/Loris that referenced this pull request Dec 19, 2018
I don't see any way the failures in aces#4002 could possibly be related to just
moving around where things are called.
@driusan driusan removed the "Help! I don't understand Travis!" PR is having a beef with TRAVIS. Someone needs to help label Dec 19, 2018
@driusan
Copy link
Collaborator Author

driusan commented Dec 19, 2018

@PapillonMcGill Can you re-review?

@@ -51,7 +51,7 @@ class UserPermissions
function select($username)
{
// create DB object
$DB =& Database::singleton();
$DB = \Database::singleton();
Copy link
Contributor

Choose a reason for hiding this comment

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

Not relevant to this PR but also.... this is nice.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, it was throwing a database exception on that line and I thought that might be the cause..

Copy link
Contributor

@PapillonMcGill PapillonMcGill left a comment

Choose a reason for hiding this comment

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

@johnsaigle comment would be nice but won't block.

@driusan driusan merged commit 5559913 into aces:major Dec 20, 2018
@ridz1208 ridz1208 modified the milestones: 22.0.0, 21.0.0 Dec 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Meta PR does something that organizes, upgrades, or manages the functionality of the codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants