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

Removed parameters.yml and sqlite database files from repo #1

Closed
wants to merge 1 commit into from

Conversation

norberttech
Copy link

No description provided.

@javiereguiluz
Copy link
Member

@norzechowicz thanks for your pull request. I was expecting a PR like this, but I didn't expect it that soon!

Sadly I must close this PR without merging it. The reason? This demo application must work out-of-the-box without configuring anything or executing any command. That's why we link compiled CSS/JS instead of using Assetic, we provide a SQLite database with the sample data instead of forcing the user to create the database and load fixtures, etc.

This application is for the absolute Symfony beginner, so everything must work right without doing anything. I hope you understand it :)

@norberttech
Copy link
Author

Yea I see the point of keeping the database file but not the parameters.yml, even absolute Symfony beginner needs to install dependencies and this will generate parameters.yml, but correct me if I'm wrong.

@javiereguiluz
Copy link
Member

@norzechowicz when installing this application via the symfony demo command, the installer will download a ZIP/TGZ package with all the vendors and everything included. Composer won't be executed and it won't even be necessary to download, install and test the demo application.

@norberttech norberttech deleted the cleanup branch March 24, 2015 14:51
@michalpipa
Copy link

Why not configure database in config.yml? You don't have to store database connection parameters in parameters.yml.

It is a bad practice to keep parameters.yml in VCS repository.

@norberttech
Copy link
Author

It's also a bad practice to keep whole database in repository.

@javiereguiluz
Copy link
Member

@michalpipa you are right (and @norzechowicz is also right). It's a good practice to not put parameters.yml in the repository. That's what we explain in the Best Practices book.

But this application is a very special piece of software. It's designed as a learning tool for the absolute beginner. What we want is that you execute the "symfony demo" command in the Symfony Installer ... and 10 seconds later you have downloaded and installed a full Symfony application that works without executing any command or doing any change.

@weaverryan
Copy link
Member

Hi guys!

I totally agree with Javier :). But what we can do is add comments in the code to help guide the beginners as they're discovering things.

@javiereguiluz I do have one idea I'd like your thoughts on. What if we didn't commit parameters.yml nor the database. But we did have a parameters.yml.dist (we already do) and a app/data/blog.sqlite.dist. We could then have 1 very simple "prep" step that copied both of these to parameters.yml and blog.sqlite respectively right before creating the tar. I want to keep things simpler, but this might be a nice way to keep things very very easy, but include more examples of good practices.

@javiereguiluz
Copy link
Member

@weaverryan I propose you something: let me finish the symfony demo command and all the other things to make this a reality. And once we test the actual command and the experience, let's see how we can improve it, or how much can we change it without degrading it. We won't have to wait long, because everything should be finished very soon.

@weaverryan
Copy link
Member

👍 Love that we'll be able to iterate and improve the demo very easily. So yea, no rush to do this

javiereguiluz added a commit that referenced this pull request Jun 21, 2021
…ky-bw)

This PR was squashed before being merged into the main branch.

Discussion
----------

Install stty required for tests on ubuntu-latest

In every test job we have skipped tests:
```
Run vendor/bin/simple-phpunit -v
PHPUnit 9.5.4 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.0.5
Configuration: /home/runner/work/demo/demo/phpunit.xml.dist
Warning:       Your XML configuration validates against a deprecated schema.
Suggestion:    Migrate your XML configuration using "--migrate-configuration"!

Testing
SSSS...........................................                   47 / 47 (100%)

Time: 00:14.918, Memory: 74.50 MB

There were 4 skipped tests:

1) App\Tests\Command\AddUserCommandTest::testCreateUserNonInteractive with data set #0 (false)
`stty` is required to test this command.

/home/runner/work/demo/demo/tests/Command/AddUserCommandTest.php:35

2) App\Tests\Command\AddUserCommandTest::testCreateUserNonInteractive with data set #1 (true)
`stty` is required to test this command.

/home/runner/work/demo/demo/tests/Command/AddUserCommandTest.php:35

3) App\Tests\Command\AddUserCommandTest::testCreateUserInteractive with data set #0 (false)
`stty` is required to test this command.

/home/runner/work/demo/demo/tests/Command/AddUserCommandTest.php:35

4) App\Tests\Command\AddUserCommandTest::testCreateUserInteractive with data set #1 (true)
`stty` is required to test this command.

/home/runner/work/demo/demo/tests/Command/AddUserCommandTest.php:35

OK, but incomplete, skipped, or risky tests!
Tests: 47, Assertions: 92, Skipped: 4.

Remaining indirect deprecation notices (1)

  1x: The "DAMA\DoctrineTestBundle\Doctrine\DBAL\AbstractStaticDriverV2" class implements "Doctrine\DBAL\Driver\ExceptionConverterDriver" that is deprecated.
    1x in PHPUnitExtension::executeBeforeFirstTest from DAMA\DoctrineTestBundle\PHPUnit
```

It would be good to run those tests at least on Ubuntu I think

Commits
-------

944b320 Install stty required for tests on ubuntu-latest
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.

4 participants