Skip to content

Conversation

@driusan
Copy link
Collaborator

@driusan driusan commented Mar 7, 2023

There are currently 3 methods of getting a reference to a database in LORIS, in order of preference:

  1. LorisInstance->getDatabaseConnection()
  2. NDB_Factory::singleton()->database();
  3. Database::singleton()

The only reference to 3 left is in the implementation of option 2. This removes the Database::singleton by inlineing the reference into the factory and removes all remaining references to Database::singleton (mostly from documentation and a couple unit tests.)

@driusan driusan added the "Help! I don't understand Travis!" PR is having a beef with TRAVIS. Someone needs to help label Mar 7, 2023
ridz1208
ridz1208 previously approved these changes Mar 8, 2023
Copy link
Collaborator

@ridz1208 ridz1208 left a comment

Choose a reason for hiding this comment

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

looks good other than the couple comments

@driusan driusan added Category: Cleanup PR or issue introducing/requiring at least one clean-up operation and removed "Help! I don't understand Travis!" PR is having a beef with TRAVIS. Someone needs to help labels Mar 8, 2023
@driusan
Copy link
Collaborator Author

driusan commented Mar 8, 2023

@kongtiaowang it seems like some unit tests can't connect to the database but the log has the credentials and it's consistently the same tests.. do you have any ideas?

@driusan driusan added the "Help! I don't understand Travis!" PR is having a beef with TRAVIS. Someone needs to help label Mar 8, 2023
@driusan driusan removed the "Help! I don't understand Travis!" PR is having a beef with TRAVIS. Someone needs to help label Mar 8, 2023
@driusan driusan requested a review from ridz1208 March 8, 2023 16:06
@driusan
Copy link
Collaborator Author

driusan commented Mar 8, 2023

@ridz1208 getting the tests to pass took more significant changes than expected, can you re-review?

@driusan driusan merged commit 6b80e4f into aces:main Mar 8, 2023
@ridz1208 ridz1208 added this to the 25.0.0 milestone Mar 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Category: Cleanup PR or issue introducing/requiring at least one clean-up operation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants