Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Update PHP CS Fixer to v1.7 #7460

Closed
wants to merge 3 commits into from

Conversation

keradus
Copy link
Contributor

@keradus keradus commented Apr 27, 2015

No description provided.

@Maks3w Maks3w assigned weierophinney and unassigned weierophinney May 4, 2015
@Maks3w
Copy link
Member

Maks3w commented May 4, 2015

This is supersede by #7477 and d2bda8f

Thank you anyway

@Maks3w Maks3w closed this May 4, 2015
@Maks3w Maks3w added this to the 2.4.1 milestone May 4, 2015
@keradus
Copy link
Contributor Author

keradus commented May 4, 2015

@Maks3w , I can't see any relation with #7477.
Also, on develop branch there is no caching enabled.

@Maks3w
Copy link
Member

Maks3w commented May 4, 2015

No there was not relation. Last commit in the branch do a global update.

About the caching thing if you want you can create a PR only with that. However I don't see benefits with the inminent component split

@keradus
Copy link
Contributor Author

keradus commented May 4, 2015

If sth can be done faster why not take advantages of it?
Even after component split ;)

@Maks3w
Copy link
Member

Maks3w commented May 4, 2015

I would like to hear how that cache works. With component split php-cs-fixer is only executed once and temporal files are not persisted across builds.

@Maks3w
Copy link
Member

Maks3w commented May 4, 2015

Also in the actual configuration all commands are executed in parallel at the same time so many processes are prone to rebuild the cache due race conditions.

@keradus
Copy link
Contributor Author

keradus commented May 4, 2015

One can want to run the fixer locally before sending PR.
In brief when using cache file is not trying to fix again if file itself wasn't changed and using same PHP CS Fixer version. More info available at tool's page.

@Maks3w
Copy link
Member

Maks3w commented May 4, 2015

@keradus If you think is needed please open a PR with only that feature and describe the benefits and possible issues.

@keradus
Copy link
Contributor Author

keradus commented May 4, 2015

It is not needed, just cool to have.
If you see no point of it then cool, no need for new PR.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants