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

Add check.php file for access from web #81

Merged
merged 4 commits into from
Jul 7, 2015
Merged

Add check.php file for access from web #81

merged 4 commits into from
Jul 7, 2015

Conversation

javiereguiluz
Copy link
Member

This PR takes the work made by @bocharsky-bw in #74 and applies the look-and-feel of the Symfony Demo application to better integrate it.

No errors - Before

before_ok

No errors - After

after-ok

Errors - Before

before-ko

Errors - After

after-ko

bocharsky-bw and others added 2 commits June 30, 2015 12:27
I propose to get Symfony's `config.php` configurator file and rename it to `check.php` with some minor changes in order to get ability to check required and optional environment configuration parameters directly from web browser for success run this demo project. It's like `app/check.php` file for console, but allow users to check environment directly from web browser. I think it will be useful for beginners. What do you think, guys?
@OskarStark
Copy link
Contributor

i think the symfony logo should be integrated

@bocharsky-bw
Copy link
Contributor

@javiereguiluz Thanks a lot! I like it 👍

@bocharsky-bw
Copy link
Contributor

@OskarStark There is no Symfony's logo in Symfony Demo Application, so I think it will be unnecessary for this PR.

P.S. I think Symfony's logo discussion could be open as a new issue.

@javiereguiluz
Copy link
Member Author

@OskarStark I think the Symfony logo should be reserved for the Symfony installation. This demo is independent from the framework, so I think we can change its design to match our styles.

@OskarStark
Copy link
Contributor

yes but the name Symfonyis used everywhere...

@javiereguiluz
Copy link
Member Author

@OskarStark let me put my reasoning in other words: this app is "made with Symfony" but "it's not Symfony". But let's see if other developers think we should add the logo.

@OskarStark
Copy link
Contributor

@javiereguiluz i understand your reasoning, but i think this is an app which shows best practices by the symfony community with standard symfony edition... so a logo makes it more clear.... before this pr there was a symfony logo, or?

@javiereguiluz
Copy link
Member Author

@bocharsky-bw I agree. Done. Thanks.

@bocharsky-bw
Copy link
Contributor

@OskarStark I think better to create a new issue for this discussion about Symfony's logo in order to other developers could also tell their opinion.

@bocharsky-bw
Copy link
Contributor

@javiereguiluz What about to add some instructions to README.md for this check.php usage at the beginning?

@javiereguiluz
Copy link
Member Author

@bocharsky-bw I like the idea a lot. Done in d88ef7b

@OskarStark
Copy link
Contributor

👍 for the note

@xabbuh
Copy link
Member

xabbuh commented Jul 6, 2015

What about renaming the file to config.php? Then its name is consistent with the one from the Standard Edition.

@bocharsky-bw
Copy link
Contributor

@xabbuh I think about it too, but there are no any config operations, only environment checks, so it seems to confused a bit as for me.

@javiereguiluz
Copy link
Member Author

This feature is now merged. Thank you @bocharsky-bw for starting it and thank you all for the reviews and the discussion!

@javiereguiluz javiereguiluz merged commit d88ef7b into symfony:master Jul 7, 2015
javiereguiluz added a commit that referenced this pull request Jul 7, 2015
…iereguiluz)

This PR was merged into the master branch.

Discussion
----------

Add check.php file for access from web

This PR takes the work made by @bocharsky-bw in #74 and applies the look-and-feel of the Symfony Demo application to better integrate it.

### No errors - Before

![before_ok](https://cloud.githubusercontent.com/assets/73419/8523007/1b62e224-23f3-11e5-9efc-3ba6bd874545.png)

### No errors - After

![after-ok](https://cloud.githubusercontent.com/assets/73419/8522991/147879d8-23f3-11e5-8158-006744863e24.png)

### Errors - Before

![before-ko](https://cloud.githubusercontent.com/assets/73419/8523014/22518c66-23f3-11e5-9059-c4ad9941617b.png)

### Errors - After

![after-ko](https://cloud.githubusercontent.com/assets/73419/8523017/2a22993a-23f3-11e5-89d0-18c2ced9ddc4.png)

Commits
-------

d88ef7b Added a note about in the README about the new requirements checked
6972191 Minor change to allow sub-directory installation
2295159 Add check.php file for access from web
2128178 Add check.php file for access from web
fabpot added a commit to sensiolabs/SensioDistributionBundle that referenced this pull request Jul 27, 2015
This PR was squashed before being merged into the 4.0.x-dev branch (closes #217).

Discussion
----------

Remove the web configurator

I propose to remove the web configurator. Some arguments in favor of doing it:

  * It does two totally different things: check technical requirements and update the parameters file.
  * If we remove it, it's very easy to maintain the useful requirements checker (we already did this in the Symfony Demo application; see symfony/demo#81)
  * When it was created, it was truly useful. Nowadays, its usefulness is unclear.
  * It's incredibly overengineered (again, because it was created a very long time ago).
  * If we remove it, we'll have 25 less files to manage and more than 1,000 less lines of code.

Commits
-------

0815bed Remove the web configurator
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.

None yet

4 participants