Skip to content

#838 - Removing extra validation step while creating#887

Closed
zabil wants to merge 4 commits intogocd:masterfrom
zabil:master
Closed

#838 - Removing extra validation step while creating#887
zabil wants to merge 4 commits intogocd:masterfrom
zabil:master

Conversation

@zabil
Copy link
Contributor

@zabil zabil commented Feb 11, 2015

goConfigFileDao return a valid config file. Writing to the output stream to create the document root can skip the validation.

@pravds pravds self-assigned this Feb 12, 2015
@arikagoyal arikagoyal added the high label Mar 2, 2015
@mdaliejaz mdaliejaz added high and removed high labels Mar 2, 2015
@mdaliejaz
Copy link
Contributor

mdaliejaz commented Mar 2, 2015

Reviewed it. Looks fine :)
Will however wait for the code walkthrough to happen before merging.

houndci-bot
houndci-bot reviewed Mar 5, 2015
View reviewed changes
server/webapp/WEB-INF/rails.new/app/controllers/cctray_controller.rb Outdated
Copy link

@houndci-bot houndci-bot Mar 5, 2015

Choose a reason for hiding this comment

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

Keep a blank line before and after private.

@arvindsv
Copy link
Member

arvindsv commented Mar 6, 2015

@zabil, @mdaliejaz: It seems like a wrong merge has happened. It's showing too many commits in this PR.

@mdaliejaz
Copy link
Contributor

mdaliejaz commented Mar 8, 2015

Yeah, I think the PR has been rebased.
@zabil What about sending a fresh PR with only the relevant commits?

@zabil
Copy link
Contributor Author

zabil commented Mar 9, 2015

Did a rebase. Screwed up the history. Resending this.

@zabil zabil closed this Mar 9, 2015
@zabil
Copy link
Contributor Author

zabil commented Mar 9, 2015

Yeah I screwed up during rebase.

New one up here.
#941

On Sun, Mar 8, 2015 at 11:53 AM, Md Ali Ejaz notifications@github.com
wrote:

Yeah, I think the PR has been rebased.
@zabil https://github.com/zabil What about sending a fresh PR with only
the related commits?

—
Reply to this email directly or view it on GitHub
#887 (comment).

@arvindsv arvindsv mentioned this pull request Mar 9, 2015
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

high

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@zabil @mdaliejaz @arvindsv @houndci-bot @pravds @arikagoyal