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

Refactor SQLiteDataConverter and DataConverter #18

Merged
merged 11 commits into from
Sep 28, 2021

Conversation

asogaard
Copy link
Collaborator

Working on #11 as well as a general refactor and simplification of these two classes. Creating a PR draft, as the work is still underway, in order to be able to start having a discussion.

@asogaard asogaard self-assigned this Sep 24, 2021
@asogaard asogaard marked this pull request as ready for review September 28, 2021 06:57
@asogaard asogaard linked an issue Sep 28, 2021 that may be closed by this pull request
@asogaard
Copy link
Collaborator Author

Hi @RasmusOrsoe,
The PR now goes some way towards what it says on the tin (refactoring the two classes), and perhaps most importantly it addresses issue #11. To avoid ballooning this PR any further I propose not making any more changes (except for requests during review) before we have had a chance to discuss and merge this first.

Copy link
Collaborator

@RasmusOrsoe RasmusOrsoe left a comment

Choose a reason for hiding this comment

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

Hi! Looks nice.

I can see that the i3/gcd file search has changed quite a bit and I understand very little of the new code. Are we 100% sure its identical?

If so, lets merge.

Rasmus

EDIT:

Compared the old and new code and it passed my equivalence test.

@asogaard
Copy link
Collaborator Author

Thanks for reviewing, @RasmusOrsoe! I have checked, and the two should agree, but I will prepare a double-check just to be sure.

@asogaard
Copy link
Collaborator Author

For reference, the new function(s) make two changes:

  • They explicitly ignore files that don't resemble I3 or GCD files.
  • Return all I3 files in a certain directory, regardless of depth.

Copy link
Collaborator

@RasmusOrsoe RasmusOrsoe left a comment

Choose a reason for hiding this comment

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

i3/gcd finder tested and works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move parallelised functions to private methods in SQLiteDataConverter
2 participants