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

release framework script clean up #1606

Merged
merged 6 commits into from
Dec 15, 2018

Conversation

samsonasik
Copy link
Member

@samsonasik samsonasik commented Dec 12, 2018

@jim-parry I think the flow before distribute framework files as below:

  • clean up dist/framework/* with rm -rf * to ensure no unneeded old files not removed.
  • cp the main directories/files ( system README.md composer.json contributing.md license.txt )

Checklist:

  • Securely signed commits

@samsonasik
Copy link
Member Author

I applied exclude composer.json from releasable variable as will be overriden by framework/composer.json in next step, and remove phpunit.xml.dist and README.md already use latest README.md copied from CI_DIR

@jim-parry
Copy link
Contributor

Appreciate your effoirts to cleanup, but...

  • The rm -f * you suggest is already there ... in the setup_repo function inside "release".
  • The set of folders to include in dist/framework is purposeful - we don't want tests but we do want applciation, public, etc
  • You suggest deleting admin/framework/readme, yet we do want to provide for a different readme than the main repo
  • similarly, the phpunit.xml.dist is a suggested starting point for phpunit testing, but targeting a developer not a contributor, i.e. not testing the system stuff

The extra semi-colon is a valid point, though not worthy of a PR on its own

@samsonasik
Copy link
Member Author

samsonasik commented Dec 12, 2018

I reverted the files deletion. For releasable variable, the composer.json and README.md still not needed as will always be overriden by admin/framework/composer.json and admin/framework/README.md. How about add .gitattributes in admin/framework directory ?

@jim-parry
Copy link
Contributor

.gitattributres should be neither here nor there. I took the approach of only copying in what should be part of the framework download, rather than copying most/all of it and then excluding with .gitattributes. trying to keep things simple.
What would .gitattributes add?

@samsonasik
Copy link
Member Author

For example, I create project via appstarter command:

composer create-project -sdev codeigniter4/appstarter 

The project with includevendor/codeigniter4 directory will look like:

├── LICENSE
├── README.md
├── application
├── composer.json
├── composer.lock
├── contributing.md
├── env
├── license.txt
├── public
├── spark
├── writable
└─- vendor/codeigniter4
    ├── framework
         ├── LICENSE
         ├── README.md
         ├── application
         ├── composer.json
         ├── contributing.md
         ├── env
         ├── license.txt
         ├── phpunit.xml.dist
         ├── public
         ├── spark
         ├── system
         └── writable

By use .gitattributes, we can have cleaner structure, eg:

├── LICENSE
├── README.md
├── application
├── composer.json
├── composer.lock
├── contributing.md
├── env
├── license.txt
├── public
├── spark
├── writable
└─- vendor/codeigniter4
    ├── framework
         ├── README.md
         ├── composer.json
         ├── contributing.md
         ├── license.txt
         ├── system

so, no more application or public directories and other existing same directories/files inside vendor/codegniter4/framework that already in root project.

@jim-parry
Copy link
Contributor

Hmm - I thought I mentioned earlier that application & public, and the other files there, would be useful for comparison if the framework is updated for someone who already has the starter installed. What is needed perhaps is a difference report between the new release and the previous or locally installed one.
What I am trying to prevent is a developer having to go back to the repo so see what is different in app/Config/Something, which they have modified locally. Yes, they "pay" for two copies, one in app/Config/Something and one in vendor/codeigniter4/framework/app/Config/Something, but it is all on their system.
I suspect there will be a number of Config changes over the next year, as v4 settles, and the community has been quite clear that a significant portion of them don't want to hunt all over the place, or have dependencies.

This might be something to bring up on the forum - just what should or shouldn't be in each distribution. They are startig points, but probably not best addressed through a PR, or a number of PRs each addressing one aspect of the distros.

@jim-parry
Copy link
Contributor

I can get the ball rollig on a forum discussion, for community input on this, but not until next week or later. My time is tight right now, and alpha.4 should be out this weekend, with the folder renaming.

@titounnes
Copy link
Contributor

Maybe we can split to difference. If a new project starts, of course nothing can be compared. Certainly different when we do an update. Only a few config files need to be compared.

@jim-parry
Copy link
Contributor

"split to difference"? I don't understand.
It may not be just a few config files, but potentially everything in application & public, that might have changed.
git has a compare verb, if you know the commit hashes; I think it has comething to compare the hashes of tags too, which might be what we are looking for.

@titounnes
Copy link
Contributor

I mean, like when we use rsync. For example we exclude config/database.php during an update

@jim-parry jim-parry merged commit cb0f8a4 into codeigniter4:develop Dec 15, 2018
@samsonasik samsonasik deleted the release-framework-script branch December 15, 2018 08:49
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.

3 participants