Skip to content
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

Source config endpoint #692

Closed
wants to merge 4 commits into from
Closed

Source config endpoint #692

wants to merge 4 commits into from

Conversation

Oludare96
Copy link

Creation of SourceConfig endpoint

Copy link
Contributor

@laurenbarker laurenbarker left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Just a couple small naming things. We follow the PEP 8 style guide which specifies naming conventions for function and class names among other things.

There are Atom linters that will highlight unconventional syntax/formatting in the editor. flake8 is just a wrapper around PEP8 and is part of our test suite. Linters are super helpful and I would recommend installing one 💯

@@ -5,3 +5,4 @@
from .registration import * # noqa
from .schema import * # noqa
from .banner import * # noqa
from .sourceConfig import * # noqa
Copy link
Contributor

Choose a reason for hiding this comment

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

Tiny changes:

  • sourceConfig --> source_config
  • two space before the comment

api/urls.py Outdated
@@ -86,6 +86,7 @@ def register_url(self, subclass, viewset):
register_route(r'rawdata', views.RawDatumViewSet)
register_route(r'user', views.ShareUserViewSet)
register_route(r'sources', views.SourceViewSet)
register_route(r'sourceConfig', views.SourceConfigViewSet)
Copy link
Contributor

Choose a reason for hiding this comment

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

sourceConfig --> source_config

class SourceConfigViewSet(viewsets.ReadOnlyModelViewSet):
serializer_class = SourceConfigSerializer

def get_queryset(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Since there isn't any special logic, we can just define queryset instead of overriding the get_queryset function. See the last example in DRF ViewSet example section

Copy link
Contributor

@laurenbarker laurenbarker Jun 28, 2017

Choose a reason for hiding this comment

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

Also, the file should be renamed 👍

… into SourceConfig-endpoint

* 'develop' of https://github.com/CenterForOpenScience/SHARE:
  [Fix] Correct harvester scheduling
  [SHARE-924][Fix] Make RawDataJanitor actually feasible
  [Fix] Don't use Django's delete
@laurenbarker
Copy link
Contributor

This PR will be included in #700, thank you!

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