Add status-provider Flag #38
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Travis tests have failedHey @vex8, 1st Buildcoala --non-interactive -VTravisBuddy Request Identifier: 0fd95ca0-f085-11e8-a427-d38b467abfda |
suggoitanoshi
force-pushed
the
status-provider
branch
from
c2711bc
to
d31c62a
Compare
Travis tests have failedHey @vex8, 1st Buildcoala --non-interactive -VTravisBuddy Request Identifier: ba7bc3f0-f085-11e8-a427-d38b467abfda |
suggoitanoshi
force-pushed
the
status-provider
branch
2 times, most recently
from
016ebd9
to
d7aef45
Compare
kx-chen
reviewed
Nov 25, 2018
org_status/org_status.py
Outdated
| @@ -87,6 +87,7 @@ def get_argument_parser(): | |||
| parser.add_argument('--export-repos', type=str) | |||
| parser.add_argument('--format', type=str, default='gitman') | |||
| parser.add_argument('--check-providers-only', action='store_true') | |||
| parser.add_argument('--status-provider', type=str) | |||
There was a problem hiding this comment.
--status-providers? I think it'd be more clear.
org_status/org_status.py
Outdated
| @@ -36,7 +36,7 @@ def generate_fetch_jobs(org_strings): | |||
| for supported_host in get_all_supported_hosts(): | |||
| if host == supported_host.HostName: | |||
| yield (supported_host, org) | |||
| raise StopIteration | |||
| # raise StopIteration | |||
There was a problem hiding this comment.
Either get rid of the code, or leave it in, but please don't have it as a comment.
org_status/org_hosts/gitlab.py
Outdated
| branch=branch) | ||
|
|
||
| repo_status = [] | ||
| for i in enumerate(self._status_provider): |
There was a problem hiding this comment.
Originally, the hacky if statement was added in #19 as a temporary solution. This isn't the ideal way to check each StatusProvider.
Please see #19 (comment) (and the other comments on there) for ideas on how to do it.
org_status/org_hosts/gitlab.py
Outdated
| if Status.PASSING in repo_status: | ||
| repo_status = Status.PASSING | ||
| # if statuses are identical, just return one of them | ||
| elif repo_status[0] == repo_status[1]: |
There was a problem hiding this comment.
This if statement will fail if there is more than one result. The entire if is not a good solution; I think it needs to be re thought.
org_status/org_status.py
Outdated
| status_provider.append(supported_provider) | ||
| found = True | ||
| break | ||
| if found: |
There was a problem hiding this comment.
Why not just check for if not found?
org_status/org_hosts/github.py
Outdated
| repo_status = Status.PASSING | ||
| # if statuses are identical, just return one of them | ||
| elif repo_status[0] == repo_status[1]: | ||
| if len(repo_status) > 1: |
There was a problem hiding this comment.
We shouldn't be repeating the same code again in gitlab.py. See other comments for suggestions instead of these if's.
ksdme
reviewed
Nov 25, 2018
There was a problem hiding this comment.
I think the best way to implement this functionality would be to simply extend syntax to submit a task to include the status provider to use, say to something like travis@gitlab:coala or some other proper alternate instead of just gitlab:coala. We can prevent these kinds of hacky solutions by simply ensuring that an instance of OrgHost is only attached to one StatusProvider which can be configured during object construction. This way we can integrate well with the current infrastructure and not break anything.
suggoitanoshi
force-pushed
the
status-provider
branch
from
d7aef45
to
27e65fe
Compare
Travis tests have failedHey @vex8, 1st Buildcoala --non-interactive -VTravisBuddy Request Identifier: 8d5df590-f22f-11e8-bef6-4798215d728f |
suggoitanoshi
force-pushed
the
status-provider
branch
from
27e65fe
to
5e54047
Compare
Travis tests have failedHey @vex8, 1st Buildcoala --non-interactive -VTravisBuddy Request Identifier: 75d4ea40-f230-11e8-bef6-4798215d728f |
suggoitanoshi
force-pushed
the
status-provider
branch
from
5e54047
to
e9f20a6
Compare
Travis tests have failedHey @vex8, 1st BuildpytestTravisBuddy Request Identifier: 7adf3d50-f231-11e8-bef6-4798215d728f |
suggoitanoshi
force-pushed
the
status-provider
branch
from
e9f20a6
to
33cdc3f
Compare
Travis tests have failedHey @vex8, 1st Buildcoala --non-interactive -VTravisBuddy Request Identifier: 8f4d48d0-f237-11e8-bef6-4798215d728f |
This commit adds a pattern to determine status-provider. The status-provider is determined by @ at the beginning of `orgs` argument Closes ksdme#17
suggoitanoshi
force-pushed
the
status-provider
branch
from
33cdc3f
to
c05945f
Compare
|
Requires more work, please be on stand by. I'll review it a bit later. |
|
Alright, for now I'll assess my code once more and see what I can do.
…On Tue, Nov 27, 2018, 20:51 Kilari Teja ***@***.*** wrote:
Requires more work, please be on stand by. I'll review it a bit later.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#38 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AKF4UrnIZTjYHYSwhej3NhrQMBQxf9Vmks5uzUNOgaJpZM4YxwW7>
.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds a status-provider flag to mix and match between Status
Providers and Hosts, while falling back to defaults.
Closes #17