-
Notifications
You must be signed in to change notification settings - Fork 0
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
Introduce create ticket script #1
Conversation
3f6975e
to
931600d
Compare
42e1c1a
to
25d5236
Compare
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.
I am unclear which tests are written to cover all the code. Are these the main
functions?
I suggest you create a tests
folder` and create there test modules using pytest that import the production code.
Please note that for deployment, it is recommended to use a setup file: https://docs.python.org/3/distutils/setupscript.html If this is for internal use only, then you do not need it. |
Thanks |
92db94a
to
1ec4352
Compare
Thanks addressed comments, new files maybe worth to remove github from Ticket class, and just send repo to it, since its the only thing it uses |
https://github.com/oshoval/github2jira/compare/25d5236..b717bdf |
Refactor the issue class |
Added unit tests for Issue class |
3d89f16
to
c641dee
Compare
c9b8eda
to
dad91a6
Compare
addressed rest of the comments |
6ded72e
to
3e37b7e
Compare
addressed comment |
3e564a1
to
4f3a5ab
Compare
added some more tests, and tests/common lib |
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.
The production code seems good, I only had small comments about it, nothing major.
The unit tests require more work.
Although I will not insist on it, please note that the unit tests are still suppose to check scenarios and not individual methods. I mean:
- checking that you can filter the relevant github issues.
- Opening a Jira ticket, based on some data.
- Failing to open a Jira ticket or contacting github.
- Not finding any relevant issues on github to process.
Etc...
Thanks, |
addressed most of the comments |
Thanks, addressed rest of the comments (unless unresolved) |
85dfe8e
to
1fedf46
Compare
since some Jira servers will deprecate basic auth and move to PAT, support it |
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.
some of those sound as e2e,
I do not think they are, as the intention is to unit test scenarios that have meaning (if possible).
I prefer to not develop this project any deeper right now in aspect of tests, and provide as is.
In case we decide we can improve it later.
Not investing in tests is raising the risk you still have bugs or worse, that you may introduce bugs in the future.
As you are the one who needs to maintain this, it is your burden to carry so I will not push you to do anything beyond what you want.
Some aren't possible. For example Anyhow as we said, I prefer to first publish an initial version, and upon needs to develop it, |
The script automates github issue to Jita ticket mirroring. Signed-off-by: Or Shoval <[email protected]>
thanks, addressed comments |
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.
Nice work!
Thanks for the great review Edy |
The script automates github issue to Jita ticket mirroring.