Skip to content

Conversation

peterbaumert
Copy link
Contributor

Solve #33

@codecov
Copy link

codecov bot commented Aug 17, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.14% 🎉

Comparison is base (8d58695) 97.86% compared to head (7a6de12) 98.01%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #90      +/-   ##
==========================================
+ Coverage   97.86%   98.01%   +0.14%     
==========================================
  Files          17       17              
  Lines         703      705       +2     
==========================================
+ Hits          688      691       +3     
+ Misses         15       14       -1     
Files Changed Coverage Δ
joeflow/models.py 98.66% <100.00%> (+<0.01%) ⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Owner

@codingjoe codingjoe left a comment

Choose a reason for hiding this comment

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

Hi @peterbaumert,

Thank you for your contribution. I always love, when people come up with excellent improvements to my packages.

Regarding your suggestion, I believe being able to adapt the direction is a splendid idea. However, I am hesitant to add settings for everything. I believe in most cases, inheritance is the more Pythonic approach to adapting behavior changes.

Might I suggest making direction a class attribute? As an added bonus, this would allow you to alter directions per individual workflow.

Best!
Joe

@codingjoe
Copy link
Owner

Maybe also go ahead and add a small test to prevent any regression.

@peterbaumert
Copy link
Contributor Author

Hi @peterbaumert,

Thank you for your contribution. I always love, when people come up with excellent improvements to my packages.

Regarding your suggestion, I believe being able to adapt the direction is a splendid idea. However, I am hesitant to add settings for everything. I believe in most cases, inheritance is the more Pythonic approach to adapting behavior changes.

Might I suggest making direction a class attribute? As an added bonus, this would allow you to alter directions per individual workflow.

Best!

Joe

Yeah that would have made sense now that you mention it :D

Will look into it those days.

The test part will be something that I need to learn about first. Never did it before 🙄

@peterbaumert
Copy link
Contributor Author

Hi @codingjoe I added a test, but I am not 100% sure if that is sufficient since it's my first time with tests :D Would be great if you could have a look

Copy link
Owner

@codingjoe codingjoe left a comment

Choose a reason for hiding this comment

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

Wonderful, took the liberty and added a commit. I added documentation and a small improvement to your test. Thanks again, for the contribution!

@codingjoe codingjoe merged commit 0906fb5 into codingjoe:main Aug 29, 2023
@codingjoe codingjoe changed the title make graph direction configurable Add class attribute to change the graph direction Aug 29, 2023
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