-
Notifications
You must be signed in to change notification settings - Fork 11
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
Small fixes #36
base: master
Are you sure you want to change the base?
Small fixes #36
Conversation
oxanayoxana
commented
Nov 16, 2019
- Updated Slack link in readme
- There was a problem while trying to edit your own project - fixed it
- In navbar 'Sign in with Github ' link was broken, because now you can not just sign, you have to chose your role. Don't know how to fix it properly - any ideas? For now I just made a redirect to the root page, so that it doesn't throw error.
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.
Check out my feedback. Let me know anything wrong/ not clear about them
def repo | ||
@repos.map(&:name) | ||
def fetch_repo | ||
@repo = @repos.map(&: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.
This method returns an array of names, right?
Why does it called in singular fetch_repo?
end | ||
|
||
def repos | ||
def fetch_repos | ||
@client = Octokit::Client.new |
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.
Can we move all these 3 lines in 1 ServiceObject?
For example in the end we would get the following:
class FetchRepo
def call
# client / github user fetched
client.repos(@github_user.login, query: { type: 'owner', sort: 'asc' })
end
end
And in the controller:
@repos = FetchRepos.call
What do you think about it?
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.
@deny7ko look, I've managed to do that! ) But why the changes don't show up hmm...
end | ||
|
||
def edit; end | ||
|
||
def create | ||
@project = Project.new(project_params) |
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.