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

0005 - Add ADR about access rules for Symfony controllers #1

Conversation

matks
Copy link
Contributor

@matks matks commented Jan 22, 2019

No description provided.

## Consequences

We need to update all controllers already built so they follow this rule.
We need to make sure controllers that will be added will follow this rule.
Copy link
Member

Choose a reason for hiding this comment

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

This should be reflected in an issue in the prestashop specs repository and linked here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

An issue or a PR ?

Copy link
Member

Choose a reason for hiding this comment

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

An issue, specs are to be written by @PrestaShop/prestashop-product-team

Choose a reason for hiding this comment

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

@matks @eternoendless We need more information and context to be able to write specs

Copy link
Member

Choose a reason for hiding this comment

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

That information should be provided in said issue 🙂

@eternoendless
Copy link
Member

0001 should be in a separate PR, shouldn't it?

@matks
Copy link
Contributor Author

matks commented Jan 22, 2019

0001 should be in a separate PR, shouldn't it?

Done #2

@matks matks force-pushed the add-ADR-about-access-rules-for-sf-controllers branch from a46cdc7 to fc12af9 Compare January 23, 2019 09:19
@matks
Copy link
Contributor Author

matks commented Jan 23, 2019

PR updated with feedbacks handled

0002-define-acl-rules-for-symfony-pages.md Outdated Show resolved Hide resolved
## Consequences

We need to update all controllers already built so they follow this rule.
We need to make sure controllers that will be added will follow this rule.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
We need to make sure controllers that will be added will follow this rule.
We need to make sure controllers that will be added will follow this rule (see [issue](https://github.com/PrestaShop/prestashop-specs/issues/1)).

We need to update all controllers already built so they follow this rule.
We need to make sure controllers that will be added will follow this rule.

See issue https://github.com/PrestaShop/PrestaShop/issues/12270
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
See issue https://github.com/PrestaShop/PrestaShop/issues/12270

We need to make sure controllers that will be added will follow this rule.

See issue https://github.com/PrestaShop/PrestaShop/issues/12270
See specs https://github.com/PrestaShop/prestashop-specs/issues/1
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
See specs https://github.com/PrestaShop/prestashop-specs/issues/1

@eternoendless eternoendless changed the title Add ADR about access rules for Symfony controllers 0005 - Add ADR about access rules for Symfony controllers Aug 21, 2020
@matks
Copy link
Contributor Author

matks commented Aug 26, 2021

ADR process is heavy and costly so I close this PR as the cost/benefit factor is not high enough.

@matks matks closed this Aug 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants