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.

Sign up for GitHub

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

Jump to bottom

Require summary in policy manifests and add it to the ones that do not have one #655

Merged
merged 4 commits into from
Mar 14, 2018
Merged

Require summary in policy manifests and add it to the ones that do not have one #655

merged 4 commits into from
Mar 14, 2018

Conversation

davidor
Copy link
Contributor

@davidor davidor commented Mar 13, 2018 •
edited
Loading

This PR makes summary mandatory in the policy manifests.
The reason is that users of the UI might be confused if they see a blank space in the list of policies instead of a summary.

@davidor davidor requested a review from mikz March 13, 2018 18:09
@mikz
Copy link
Contributor

mikz commented Mar 13, 2018

Maybe we should require the summary then?

@davidor
Copy link
Contributor Author

davidor commented Mar 13, 2018

Yes, I think we should make summary (shorter) required and description (longer) optional.

@vramosp
Copy link

vramosp commented Mar 13, 2018

UI limits summary at 75 characters I believe.

@davidor
Copy link
Contributor Author

davidor commented Mar 13, 2018

@vramosp We are enforcing that in our manifest-schema.json too: https://github.com/3scale/apicast/blob/69ff05c03e934daf1fa3a9b2cfb4231773b5a22a/gateway/src/apicast/policy/manifest-schema.json#L42

@davidor davidor changed the title Add summary to the token introspection policy manifest Require summary in policy manifests and add it to the ones that do not have one Mar 13, 2018
@davidor davidor requested review from mikz and removed request for mikz March 13, 2018 18:34
@mikz mikz requested a review from didierofrivia March 13, 2018 21:26
@didierofrivia
Copy link
Member

didierofrivia commented Mar 14, 2018 •
edited
Loading

As we talked yesterday with @davidor and @thomasmaas, we decided summary should be mandatory, and description optional.

@mikz mikz merged commit b9d2b62 into master Mar 14, 2018
@mikz mikz deleted the token-introspection-manifest-summary branch March 14, 2018 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@didierofrivia didierofrivia didierofrivia approved these changes

@mikz mikz mikz approved these changes

@thomasmaas thomasmaas thomasmaas approved these changes

Assignees

@davidor davidor

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@davidor @mikz @vramosp @didierofrivia @thomasmaas