fix: handle if ORGANIZATION not provided #32
Conversation
Fixes github#27 Use each repo's org from REPOSITORY list if ORGANIZTION not provided This makes sense because the repositories in REPOSITORY could have different organizations - [x] utilize repo's owner/org when ORGANIZTION not provided - [x] show the prefixed org is set when parsing REPOSITORY env var - [ ] additional testing Signed-off-by: jmeridth <jmeridth@gmail.com>
`black` linting Signed-off-by: jmeridth <jmeridth@gmail.com>
when the user provides the org in the org/repo comman delimited list of REPOSITORY env var, we still need to validate that the org is an org and not a user Signed-off-by: jmeridth <jmeridth@gmail.com>
Signed-off-by: jmeridth <jmeridth@gmail.com>
|
|
cleanowners.py
Outdated
| if not gh_org: | ||
| print(f"Organization {organization} not found") | ||
| return |
There was a problem hiding this comment.
Let me know if I'm reading this incorrectly, but wouldn't this if statement end the program if organization is not set and therefor gh_org returns null?
There was a problem hiding this comment.
Although the test that passes in only a repository list and a organization=None passes so maybe that means I'm wrong here. 🤷🏻
There was a problem hiding this comment.
I'll double check it. Thank you for asking.
There was a problem hiding this comment.
@zkoppert yes, it would. I was being presumptuous.
The combinatorics on this based on the code:
-
(ORGANIZATION and REPOSITORY provided) or (ORGANIZATION not provided and REPOSITORY provided)
- the repository list from REPOSITORY is used
-
ORGANIZATION provided and REPOSITORY not provided
- we use repositories related to the ORAGANIZATION if the organization is valid
- if organization invalid throw error and exit
-
ORGANIZATION not provided and REPOSITORY not provided
- throw error telling user, handled in environment variable parsing.
In situation 2, if the ORGANIZATION proves to not be an actual ORGANIZATION, we should exit the app, correct?
working on a coding change. Great catch @zkoppert
There was a problem hiding this comment.
pushed up new change to do better checking (I hope). Added new test for the get_org function, when org invalid. Also added black linter to make lint 😄
There was a problem hiding this comment.
Ok, Your summary above makes sense and is how we want it to work. I'll take a peek at the code now.
Signed-off-by: jmeridth <jmeridth@gmail.com>
- [x] check if org valid and if REPOSITORY set instead - [x] update `make lint` to include black - [x] add black to requirements-test.txt Signed-off-by: jmeridth <jmeridth@gmail.com>
Signed-off-by: jmeridth <jmeridth@gmail.com>
Fixes #27
Pull Request
Proposed Changes
Use each repo's org from REPOSITORY list if ORGANIZTION not provided
This makes sense because the repositories in REPOSITORY could have different organizations
make lintto include blackWe might want additional testing, up to @zkoppert.
Readiness Checklist
Author/Contributor
make lintand fix any issues that you have introducedmake testand ensure you have test coverage for the lines you are introducingReviewer
bug,documentation,enhancement,infrastructure, orbreaking