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.

Sign up for GitHub

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

Jump to bottom

Fixed two issues related to feature extraction #261

Merged
merged 3 commits into from
Feb 11, 2022
Merged

Fixed two issues related to feature extraction #261

merged 3 commits into from
Feb 11, 2022

Conversation

ppanopticon
Copy link
Member

Changes:

  • Initialisation of Writers now takes place in construction
  • DatabaseConfig.getSelectorSupplier() now respects selector config instead of writer config.

@ppanopticon
Fixed two issues related to extraction:
1e7dbfc 
- Initialisation of Writers now takes place in construction
- DatabaseConfig.getSelectorSupplier() now respects selector config instead of writer config.
@lucaro
Copy link
Member

lucaro commented Feb 11, 2022

Is the init() method still used for any of the writers? If not, we might as well get rid of it.

Spiess
Spiess previously approved these changes Feb 11, 2022
View reviewed changes
Copy link
Member

@Spiess Spiess left a comment

Choose a reason for hiding this comment

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

Works great, thanks for fixing so quickly.

@ppanopticon
Copy link
Member Author

Is the init() method still used for any of the writers? If not, we might as well get rid of it.

I agree! But I believe that this should be part of the refactoring proposed in #251. Since this is a larger undertaking, and the issues at hand are currently blockers for @Spiess, I'd suggest to move forward without that change at this point.

@ppanopticon ppanopticon merged commit 0930826 into master Feb 11, 2022
@Spiess Spiess deleted the fix/rwinitialization branch February 11, 2022 11:30
silvanheller pushed a commit that referenced this pull request Jun 10, 2022
@ppanopticon
Fixed two issues related to feature extraction (#261)
166e3fa 
* Fixed two issues related to extraction:

- Initialisation of Writers now takes place in construction
- DatabaseConfig.getSelectorSupplier() now respects selector config instead of writer config.

* Bumped version.

Former-commit-id: 0930826
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lucaro lucaro lucaro approved these changes

@Spiess Spiess Spiess left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ppanopticon @lucaro @Spiess