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

[#233 #17]: Rethink configuration storage at project level #250

Merged
merged 8 commits into from
May 10, 2020
Merged

[#233 #17]: Rethink configuration storage at project level #250

merged 8 commits into from
May 10, 2020

Conversation

MacFJA
Copy link
Contributor

@MacFJA MacFJA commented May 3, 2020

  • Read configuration from .phive and current working directory (phars + auth)
  • Update skel command
  • Add warning in output if both configuration exist
  • Add project level repositories
  • Copy phive.xml to .phive/phars.xml

@MacFJA MacFJA mentioned this pull request May 3, 2020
Closed
@MacFJA MacFJA marked this pull request as draft May 3, 2020 16:26
@MacFJA MacFJA marked this pull request as ready for review May 3, 2020 17:46
@MacFJA
Copy link
Contributor Author

MacFJA commented May 3, 2020

I still need to add more test

theseer
theseer requested changes May 3, 2020
View reviewed changes
Copy link
Member

@theseer theseer left a comment

Choose a reason for hiding this comment

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

Very nice work! Thanks a lot - again :)

src/Factory.php Outdated Show resolved Hide resolved
src/commands/help/help.md Outdated Show resolved Hide resolved
src/commands/migrate/MigrateCommand.php Outdated Show resolved Hide resolved
src/commands/migrate/MigrateCommandConfig.php Outdated Show resolved Hide resolved
src/services/migration/HomePharsXmlMigration.php Outdated Show resolved Hide resolved
src/shared/config/AuthXmlConfigFileLocator.php Outdated Show resolved Hide resolved
src/shared/exceptions/ExtensionsMissingException.php Outdated Show resolved Hide resolved
src/Factory.php Outdated Show resolved Hide resolved
tests/unit/shared/config/fixtures/doubleConfig/.phive/auth.xml Outdated Show resolved Hide resolved
tests/unit/shared/config/fixtures/doubleConfig/phive-auth.xml Outdated Show resolved Hide resolved
@theseer
Copy link
Member

theseer commented May 5, 2020

Very nice :)

Looks like we can almost merge this and finally make a release 👍

@theseer
Copy link
Member

theseer commented May 9, 2020

Released phar-io/filesystem 1.0.0, released phar-io/version 3.0.0.

Now a test in phar-io/manifest fails, so I probably introduced a BC break in 3.0.0. Once all that works again, I'll update the dependencies for phive.

@theseer
Copy link
Member

theseer commented May 9, 2020

Dependencies updated, seems to work so far ;)

@MacFJA MacFJA force-pushed the issue-233-directory-structure branch from 19feb38 to 6544cd2 Compare May 9, 2020 23:23
@MacFJA MacFJA requested a review from theseer May 9, 2020 23:25
@theseer
Copy link
Member

theseer commented May 9, 2020

Thanks! I'll look at it tomorrow.

theseer
theseer requested changes May 10, 2020
View reviewed changes
src/services/migration/HomeAuthXmlMigration.php Outdated Show resolved Hide resolved
src/services/migration/HomePharsXmlMigration.php Outdated Show resolved Hide resolved
src/services/migration/HomePhiveXmlMigration.php Outdated Show resolved Hide resolved
src/services/migration/ProjectAuthXmlMigration.php Outdated Show resolved Hide resolved
tests/unit/services/migration/HomeAuthXmlMigrationTest.php Outdated Show resolved Hide resolved
tests/unit/shared/version/GitHubVersionTest.php Outdated Show resolved Hide resolved
@MacFJA
[phar-io#233 phar-io#17]: Rethink configuration storage at project level
2f9c472 
- Read configuration from .phive and current working directory (phars + auth)
- Update skel command
- Add warning in output if both configuration exist
- Add project level repositories
- Copy phive.xml to .phive/phars.xml
@MacFJA
Fix CI (missing ant dependency) + Fix QA
9e8a434
@MacFJA
[phar-io#233] Rename phars.xml of home
ae7edee 
- Change name of ~/.phive/phars.xml to ~/.phive/database.xml
- Change name of ~/.phive/phive.xml to ~/.phive/global.xml
- Add migration service
- Add migrate command
- Add auto migration of mandatory migration
@MacFJA
Reduce objects chaining
ce99d16 
- Make Config hold direct link to all configuration files and directories
- Rename list option to status for migrate command
- Revert bad parameter rename
@MacFJA
Change auth.xml namespace + move automatique migration
f9a64f9
@MacFJA
Allow migrated file to be renamed instead of be removed
bac1f9d
@MacFJA
Add UT for migrations
c2b79ad
@MacFJA
Refactor migration to avoid duplicate code + fix UT + grammar
9f60268
@MacFJA MacFJA force-pushed the issue-233-directory-structure branch from 6544cd2 to 9f60268 Compare May 10, 2020 21:39
@MacFJA MacFJA requested a review from theseer May 10, 2020 21:44
@theseer
Copy link
Member

theseer commented May 10, 2020

Looks all good.

This change became so big, I probably missed something. :)

@theseer theseer merged commit 2db0e8c into phar-io:master May 10, 2020
@MacFJA
Copy link
Contributor Author

MacFJA commented May 10, 2020

Yes, it goes from a not so big PR to a very big PR in no time. 😅
But we made lots of additional changes (like refactor the Config class, maybe we should have made a different PR for that, to keep this one smaller 🤔)

I hope that I don't left some mistakes 🤞

@MacFJA MacFJA deleted the issue-233-directory-structure branch May 10, 2020 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@theseer theseer theseer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@MacFJA @theseer