-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add gitshelf as a format and fetch certain repos #34
base: master
Are you sure you want to change the base?
Conversation
Travis tests have failedHey @CLiu13, 1st Buildcoala --non-interactive -V
TravisBuddy Request Identifier: b3a07b70-e702-11e8-87c2-ebfa2f2e9d99 |
Travis tests have failedHey @CLiu13, 1st Buildcoala --non-interactive -V
TravisBuddy Request Identifier: 9900ba40-e703-11e8-87c2-ebfa2f2e9d99 |
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.
Looks pretty good to me! Better than what I can do ;)
Just one suggestion. (see comments)
Travis tests have failedHey @CLiu13, 1st Buildcoala --non-interactive -V
TravisBuddy Request Identifier: 981416b0-e70b-11e8-87c2-ebfa2f2e9d99 |
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.
- This PR should not close Manage a machine consumable list of all org repositories #6, please fix your commit message.
- I do not think we need another class of classes called decoders, I think you should rather rename the Encoder classes we have to be more general and then add a decoder method it can implement.
Travis tests have failedHey @CLiu13, 1st Buildcoala --non-interactive -V
TravisBuddy Request Identifier: baec6ef0-e7a4-11e8-ad5c-f14c74a66e2e |
Travis tests have failedHey @CLiu13, 1st Buildcoala --non-interactive -V
TravisBuddy Request Identifier: 934bab30-e7a5-11e8-ad5c-f14c74a66e2e |
Travis tests have failedHey @CLiu13, 1st Buildcoala --non-interactive -V
TravisBuddy Request Identifier: 4e8c0b50-eaae-11e8-b26b-2fe956a0f750 |
This adds support for gitshelf as a format for a list of repositories. This also adds support for fetching only repos that are listed in a list of repositories, as specified by --import-repos and --format. Closes ksdme#29
'repo': url, | ||
'rev': 'master'} | ||
|
||
def decode_repo_list(self, file_name): |
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.
decode_repo_list
here handles reading from the files by itself whereas encodind doesn't. I think you should have encode
method return a String of encoded data in whichever format it likes like JSON, YAML etc and likewise decode method should take a string and yield results.
with open(file_name, 'r') as file: | ||
yml_data = yaml.load(file) | ||
for repo in yml_data['sources']: | ||
yield [repo['name'], repo['repo']] |
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.
Yielding tuples'll be better.
self._status_provider = [] | ||
for i in enumerate(self.StatusProvider): | ||
self._status_provider.append(self.StatusProvider[i[0]](self._group)) | ||
|
||
self._sync = sync | ||
if sync: |
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.
Use self.sync
.
if sync: | ||
self._token = GitHubToken(token) | ||
self._org = GitHubOrganization(self._token, self._group) | ||
|
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.
Maybe something like
else: | |
self._nosync_repositories = kargs['repositories'] |
|
||
@property | ||
def repositories(self): | ||
return self._org.repositories | ||
return [repo.web_url for repo in self._org.repositories] |
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.
Please notice that if sync
is False
, this method will fail. We need an alternate build process for these classes.
This adds support for gitshelf as a format for a list
of repositories. This also adds support for fetching
only repos that are listed in a list of repositories,
as specified by --import-repos and --format.
Closes #6 and #29