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

Check installed.json #7

Closed
wants to merge 1 commit into from
Closed

Conversation

webflo
Copy link
Member

@webflo webflo commented Dec 4, 2015

No description provided.

@derhasi
Copy link
Member

derhasi commented Dec 4, 2015

I do not get your changes. What do you want to achieve with it?

@webflo
Copy link
Member Author

webflo commented Dec 4, 2015

It is the result of a lengthy in IRC with @greg-1-anderson. I was trying to prevent the update of the scaffold in CI environments. I think we want to keep the committed scaffold file in any case, even if we install a new version of the package. Because a developer did the update before and committed the files already. composer install should not update the scaffold at all if the composer.lock is present.

@webflo
Copy link
Member Author

webflo commented Dec 4, 2015

The proposed solution is not correct, i still try to locate the proper information in the composer event.

@greg-1-anderson
Copy link
Collaborator

I think #7 (comment) is on the right track. composer install only installs the scaffold files if there are no scaffold files present. composer update will update the scaffold files to the target version any time drupal/core is updated.

Since we are not trying to detect exact version changes in composer install, I think that the existing code, prior to this PR, is sufficient -- it already does the right thing for composer update. I think all we need now is logic that says, "on composer install, only install if none of the files listed in "includes" exist on disk."

greg-1-anderson added a commit to greg-1-anderson/drupal-scaffold that referenced this pull request Dec 5, 2015
… update into separate post-event handlers.
@greg-1-anderson
Copy link
Collaborator

I just added a commit to #6 to refactor this per my comment above. Next I'll add a check to see if any (all?) of the 'include' files already exist, and, if so, prevent install from updating the scaffold files. Then I think we're pretty much there, save for the remaining limitation that we can't perfectly restore the scaffolding files to the same state after a composer install with a composer.lock in place when there are newer scaffold files in HEAD.

In IRC, @webflo and I seemed to think this limitation was acceptable for now, since it is advisable for folks to commit the scaffolding files to their repository anyway.

@greg-1-anderson
Copy link
Collaborator

I think #6 now satisfies our use-cases vis-a-vis composer create-project (scaffold files downloaded) and composer install from CI (scaffold files not downloaded if checked in).

@webflo
Copy link
Member Author

webflo commented Dec 5, 2015

@greg-1-anderson @derhasi Thought about it again, and we should check the composer.lock instead of installed.json. Checks against installed.json will fail on the initial clone/install of a repo with an existing lock file.

@webflo
Copy link
Member Author

webflo commented Dec 5, 2015

We should go with #8

@greg-1-anderson
Copy link
Collaborator

Closing in favor of #8 / #20; re-open if further discussion here is desired.

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

3 participants