Skip to content

Conversation

EvanLovely
Copy link

This points the main dependencies to our forks, and alters the readme's composer create-project command to use our stuff while side-stepping Packagist (for now), and adds Travis tests.

  • Go over readme to see if any download links point to upstream repo
  • Add stuff to readme mentioning how we are a fork

Copy link

@sghoweri sghoweri left a comment

Choose a reason for hiding this comment

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

All in all looks good on my end! Tested building and serving this branch out via:

composer create-project pattern-lab/edition-twig-standard:dev-feature/update-fork --repository '{ "type": "vcs", "url": "https://github.com/drupal-pattern-lab/edition-php-twig-standard" }' your-project-name && cd $_ && php core/console --generate

php core/console --server

As an open question to everyone before we merge though, should we:
A. Include a forked version of styleguidekit-assets-default directly here and remove the dependency from https://github.com/drupal-pattern-lab/styleguidekit-twig-default/blob/master/composer.json? (my personal recommendation)
B. Update our forked https://github.com/drupal-pattern-lab/styleguidekit-twig-default/blob/master/composer.json to point at a separate fork of styleguidekit-assets-default (which, imho, doesn't make sense as a dependency since styleguidekit-assets-default is template engine agnostic)
C. Leave it as is - which I'd recommend against given the 20 open issues and 3 open PRs...

@sghoweri
Copy link

sghoweri commented May 2, 2017

image

@EvanLovely
Copy link
Author

I think C as styleguidekit-assets-default is being actively maintained by the PL Node crew: https://github.com/pattern-lab/styleguidekit-assets-default/commits/master

We could always fork and contribute later if we saw reasons.

@EvanLovely
Copy link
Author

OK, we've got comments on this in a couple places; but whatever: let's do it. Then it's less confusing as all pieces are our forks.

@EvanLovely EvanLovely merged commit ad20e10 into master May 2, 2017
@EvanLovely EvanLovely deleted the feature/update-fork branch May 2, 2017 19:43
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