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

Mb/pymetised #60

Merged
merged 13 commits into from
Nov 14, 2024
Merged

Mb/pymetised #60

merged 13 commits into from
Nov 14, 2024

Conversation

sesquideus
Copy link
Contributor

Rewrote the internal structure of the *Impl classes to work with InputSets.

  • generic and structured tests
  • improved initialization script
  • more recipes and associated input sets
  • working with new SOFs

@hugobuddel
Copy link
Contributor

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?

@sesquideus
Copy link
Contributor Author

Sorry, I was in a hurry then. Please review, I might have missed something.

Copy link
Contributor

@hugobuddel hugobuddel left a 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

init.sh Show resolved Hide resolved
@hugobuddel
Copy link
Contributor

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.

@hugobuddel
Copy link
Contributor

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.

@sesquideus sesquideus merged commit 5719be4 into main Nov 14, 2024
1 check passed
@sesquideus
Copy link
Contributor Author

What is our process @sesquideus ? With @teutoburg we usually let the person who created the PR merge it.

Gladly. I think all of these are valid points but should be postponed until after the first delivery.

@sesquideus sesquideus deleted the mb/pymetised branch November 14, 2024 21:00
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.

2 participants