Skip to content

Set up default SPs in each deployed environment#1686

Merged
zachmargolis merged 1 commit intomasterfrom
margolis-simplify-sp-registration
Sep 21, 2017
Merged

Set up default SPs in each deployed environment#1686
zachmargolis merged 1 commit intomasterfrom
margolis-simplify-sp-registration

Conversation

@zachmargolis
Copy link
Contributor

Why: Makes it easier to use the IDP and other apps

Gemfile Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

please review this PR so I can merge this to master first! 18F/identity-hostdata#1

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Gemfile Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

Choose a reason for hiding this comment

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

<%= ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yikes -- yes good catch thx

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PTAL @ 7404d58b182de55189b639e6d04dfc48ab9c600d

Copy link
Contributor

Choose a reason for hiding this comment

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

<%= ?

Copy link
Contributor

Choose a reason for hiding this comment

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

<%=

Copy link
Contributor

@brodygov brodygov left a comment

Choose a reason for hiding this comment

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

LGTM

@zachmargolis zachmargolis force-pushed the margolis-simplify-sp-registration branch from 7404d58 to 7f2a9ad Compare September 21, 2017 19:33
@zachmargolis
Copy link
Contributor Author

(doing to confirm that this works by deploying it to an environment before merging)

@brodygov
Copy link
Contributor

This explodes because we define a model class called Identity

/srv/idp/releases/chef/app/models/identity.rb:1:in `<top (required)>': Identity is not a class (TypeError)

Copy link
Contributor

@brodygov brodygov left a comment

Choose a reason for hiding this comment

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

Looks good once tests and such pass

**Why**: Makes it easier to use the IDP and other apps
@zachmargolis zachmargolis force-pushed the margolis-simplify-sp-registration branch from aea4672 to 6947c9a Compare September 21, 2017 20:16
@zachmargolis zachmargolis merged commit 8f5a231 into master Sep 21, 2017
@zachmargolis zachmargolis deleted the margolis-simplify-sp-registration branch September 21, 2017 20:38
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.

2 participants