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

Replaced organization github name and project repo fields with Project github url #286

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sai92messy
Copy link

Replaced organization github name and project repo fields with Project github url.
When user enters the Project github url the system parses the url and finds the respective Organization name and Project repo name.

@DBNess
Copy link
Member

DBNess commented Jan 27, 2015

@sai92messy this looks like a great simplification - thank you! 🎉 Could you please add a screenshot of how this has changed the Project Submission user interface? Thanks!

@sai92messy
Copy link
Author

@DBNess Great. Here is the screenshot showing the changes
codemontage-com
codemontage-change
codemontage-1-com
codemontage-1-change

@@ -13,7 +13,7 @@ class Organization < ActiveRecord::Base
attr_accessor :is_public_submission

validates_presence_of :name
validates_presence_of :github_org, if: :is_public_submission
# validates_presence_of :github_org, if: :is_public_submission
Copy link
Member

Choose a reason for hiding this comment

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

Would you please remove commented out code? Thanks!

@courte
Copy link
Member

courte commented Feb 9, 2015

@sai92messy - Thanks so much for this contribution! I love how simple and agile it is, even down to your front end changes. And you even remembered to update the tests! 💯

I've left some comments with changes I would love to see before we merge this into the code base. Let me know if you have any questions, because I'm really looking forward to merging!

@courte
Copy link
Member

courte commented Mar 11, 2015

Related to #243.

@@ -43,9 +43,16 @@ def logo_delete
@logo_delete || '0'
end

def set_github_org
if self.projects.first
self.github_org = Project.parse_git_url(self.projects.first.submitted_github_url).split('/').first

Choose a reason for hiding this comment

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

Line is too long. [104/80]
Redundant self detected.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.


def set_github_org
if projects.first
github_org = Project.parse_git_url(projects.first.submitted_github_url)

Choose a reason for hiding this comment

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

Useless assignment to variable - github_org.

private

def set_github_repo
github_repo = Project.parse_git_url(submitted_github_url).split('/').last

Choose a reason for hiding this comment

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

Useless assignment to variable - github_repo.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

…t github url, Parse github url to get organization github name and project git repo

def set_github_org
if projects.first
self.github_org = Project.parse_git_url(projects.first.submitted_github_url)

Choose a reason for hiding this comment

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

Line is too long. [82/80]

@sai92messy
Copy link
Author

@DBNess and @courte , I've made the changes that you asked for, please check the same and give your feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants