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

Continuing adding deployable label #20

Merged
merged 9 commits into from
Jul 23, 2014
Merged

Continuing adding deployable label #20

merged 9 commits into from
Jul 23, 2014

Conversation

elliothursh
Copy link
Contributor

To roll back this change, revert the merge with git revert -m 1 MERGE_SHA and perform another deploy.

URLs

QA Plan

Check tests to make sure they cover well and finish successfully

@elliothursh
Copy link
Contributor Author

@NickLaMuro check this out when you get a chance?

end

def == (obj)
return true if ((self.name == obj.name) && (self.color == obj.color))
Copy link
Collaborator

Choose a reason for hiding this comment

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

We are going to want to do some tests with this. From what we tested through the UI, it seems like only the name is the attribute that gets a uniqueness validation.

Also, is this used anywhere?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Had my second question answered.

# pull_number - number of the pull_request to add label to
def self.add_to_pull(pull_number, *labels)
label_names = get_names(build_label_array(labels))
GitHub.add_labels_to_pull(config.github_repo, pull_number, label_names )
Copy link
Collaborator

Choose a reason for hiding this comment

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

Lets to a minor switch here:

built_labels = build_label_array(labels)
GitHub.add_labels_to_pull(config.github_repo, pull_number, get_names(built_labels) )

Copy link
Collaborator

Choose a reason for hiding this comment

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

Really not a big deal I guess, so take it or leave it. Something about that looks better to me though.

@NickLaMuro
Copy link
Collaborator

@elliothursh FYI, I know that previously we were following the http://tomdoc.org/ spec for documentation. Try to keep that in mind when adding docs to methods and such.

@NickLaMuro
Copy link
Collaborator

:octocat: Merged into deployable.2014.06.26.

@NickLaMuro
Copy link
Collaborator

@elliothursh ^^ This caused a error:

error: undefined method 'name' for nil:NilClass

To reproduce, run bundle exec op deployable 20 with this branch checked out.

@NickLaMuro
Copy link
Collaborator

:octocat: Merged into deployable.2014.06.26.

@elliothursh
Copy link
Contributor Author

:octocat: Merged into deployable.2014.06.26. /cc @tst-automation

1 similar comment
@elliothursh
Copy link
Contributor Author

:octocat: Merged into deployable.2014.06.26. /cc @tst-automation

@elliothursh
Copy link
Contributor Author

:octocat: Merged into deployable.2014.06.26. /cc @tst-automation

1 similar comment
@elliothursh
Copy link
Contributor Author

:octocat: Merged into deployable.2014.06.26. /cc @tst-automation

@NickLaMuro
Copy link
Collaborator

Looks like it is working. Wait for the tests to pass and we can merge this in. 👍

@NickLaMuro NickLaMuro merged commit f663988 into master Jul 23, 2014
NickLaMuro added a commit that referenced this pull request Jul 23, 2014
@elliothursh elliothursh deleted the label-2b branch July 25, 2014 18:39
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.

None yet

2 participants