-
Notifications
You must be signed in to change notification settings - Fork 9
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
Added REST api for announcement app #63
Conversation
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.
Seems alright !
@RajuKoushik Added Serializable class for taggit . Review it again Sir. |
@yashLadha: Please deploy this branch on cloud and share link. |
Sir, @singhpratyush Is AWS free for students? |
It has a free tier for first year. |
Sir, should i deploy it to heroku or any other because for heroku i need to change settings for it and may be it won't consistent with the current system. |
I recommend using AWS. You can simply run |
Sir, currently i do not own my self a card. IS there a free alternative of it. |
@singhpratyush I can share the images of testing the api with postman. |
wsgi/odyssy/announcement/api/urls.py
Outdated
) | ||
|
||
urlpatterns = [ | ||
url(r'^$', AnnouncementListView.as_view(), name='all-announcement'), |
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.
@yashLadha sir name='all-announcement'
can we use some different name here as the name name='all_announcement'
is already used in announcement app's urls and it might get confusing later.
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.
It would be more relevant if it would be api_all_announcement
. View @Monal5031 . Now it is more connected to only the api.
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.
Yeah its good 😄
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.
It'd be good if we could see the working of this REST api. Bugs can be found that way. Views @yashLadha ?
Actually I can't setup it on AWS as i don't own a card. Is there any
alternative.
…On 25 Jun 2017 5:45 pm, "Monal Shadi" ***@***.***> wrote:
***@***.**** commented on this pull request.
It'd be good if we could see the working of this REST api. Bugs can be
found that way.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#63 (review)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ARMqT9Sn9wwHDD56fqDvDBir_eJ-lZG8ks5sHk9kgaJpZM4N1M_U>
.
|
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.
Tested it locally everything is looking good 👍
@yashLadha sir please update this PR, so we can merge it 😄 |
May be we should wait for other's to also add their review?? |
Yeah 👍 |
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.
👍
Not relevant |
Closes #50