-
Notifications
You must be signed in to change notification settings - Fork 53
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
cEP-0032: Enhance Websites Architecture #186
Conversation
cEP-0032.md
Outdated
* has atleast 1 source repositories on GitHub | ||
* has done any of the github training exercises etc. | ||
|
||
3. **Login/Signup using GitHub** |
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.
@jayvdb, needed your views on if this is required or needed to be done?
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 also have a look at this pr discussions when you get time, we needed your suggestions there too.
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 would be good. @jazzband has this for their website.
The problem will be in getting it hosted, if that is required, so definitely plan on it being an optional feature, and ensure it can be tested on your own fork with a Heroku-type hosting, because we will not deploy it until it is finished and we have time to set up an org instance of the backend.
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! we will get problem in getting it hosted because the authentication would work only if its a dynamic website which is currently being served as a static website to users
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.
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.
As I can see this is covered for Netlify. Maybe if you change the provider from heroku to Netlify, it can solve the problem immediately.
I guess deploying on netlify can be achieved by netlify provided features https://www.netlify.com/docs/authentication-providers/ 😮
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.
Yes, I haven’t worked with that, but it seems it’s easy.
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.
As I can see this is covered for Netlify. Maybe if you change the provider from heroku to Netlify, it can solve the problem immediately.
@jayvdb @sks444 @margobra8
I'm able to resolve this problem by setting two environment variables - REPOSITORY_URL and URL which are kind of necessary for coala Community repo. After adding them, the authentication is working.
Can be checked at: https://kvgarg-community.herokuapp.com/accounts/github/login/ after getting authorized it will return to homepage
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.
Will look on netlify OAuth too.
Preview: https://5d0b80bdafb942000880f8cd--determined-goldstine-6dc504.netlify.com/auth/
But If we're sticking with static website It will be difficult to maintain user-session because every time user refreshes the web-page or re-directs to some another page the session will be no more exist. And hence no benefit.
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 can have benefit if I guess Session
values can be added while a user gets logged in and use that until user is on the website.
cEP-0032.md
Outdated
organizaton has. It will allow other organization to get to know more about | ||
them. | ||
|
||
2. **Add forms for -** |
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.
I am skeptic about this section too.
Not sure if we need all these forms, needed @jayvdb confirmation.
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.
See @jazzband
@sks444 @damngamerz @margobra8 @li-boxuan Please review! |
cEP-0032.md
Outdated
1. **Display a webpage for-** | ||
1. **organization teams** | ||
|
||
This will show information about all [coala teams](https://github.com/orgs/coala/teams). |
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.
Sounds interesting, but that link is private and only available to the community members. Not sure if it is a good idea to make it public.
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.
coala/community#12 (comment)
Took idea from coala/community#12
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.
the web-page will be showed to just logged-in users, now
cEP-0032.md
Outdated
1. **Display a webpage for-** | ||
1. **organization teams** | ||
|
||
This will show information about all [coala teams](https://github.com/orgs/coala/teams). |
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.
That link won't work if visited outside of coala, change accordingly. Maybe only display info that is public, such as org members
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! It won't work for a non-member. Will make that change
Sorry! I took that suggestion in some wrong direction. @jayvdb
proposed the idea of having a page for all teams and I too think that atleast newcomers, developers and admins
team must have a page.
@sks444 @margobra8 Need review! |
The commits add the cEP explaining about the project to be implemented in the upcoming GSoC'19 phases. Closes coala#185
Closes #185