-
Notifications
You must be signed in to change notification settings - Fork 2
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
Mb/pymetised #60
Mb/pymetised #60
Conversation
Looks good! I'm not sure whether we agreed upon a process. You didn't assign a reviewer, so I'm not sure what process you have in mind. Shall I/you just merge this? |
Sorry, I was in a hurry then. Please review, I might have missed something. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! Thanks @sesquideus . I couldn't fully review this, but since it is clearly an improvement, let's merge this
Note that there is technically not necessary a one-to-one correspondence between the tag, the category, and the filename. But for now this is fine. |
What is our process @sesquideus ? With @teutoburg we usually let the person who created the PR merge it. I like that process, because usually most of the comments I give are not showstoppers, and this lets the author of the PR decide whether it is worth it to make the changes now or later. |
Gladly. I think all of these are valid points but should be postponed until after the first delivery. |
Rewrote the internal structure of the
*Impl
classes to work withInputSets
.