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

careful treatment of string PHPExcel in migrator #609

Closed
wants to merge 1 commit into from

Conversation

blunket
Copy link
Contributor

@blunket blunket commented Jul 20, 2018

Fixes #598

This is:

  • a bugfix
  • a new feature

Checklist:

Why this change is needed?

This PR resolves #598 by using a stricter regex when replacing the plain string 'PHPExcel' in the migration tool.

@blunket
Copy link
Contributor Author

blunket commented Jul 20, 2018

I truly don't understand why the build failed, but I'm open to make any changes as necessary.

@artrz
Copy link

artrz commented Jul 20, 2018

The removed line before continue has to be there, it's part of the code style rules. Run the fixer locally.

@blunket blunket force-pushed the Migration-Issue-598 branch from 0ddab94 to 350a5a7 Compare July 20, 2018 18:05
@blunket
Copy link
Contributor Author

blunket commented Jul 20, 2018

Thank you very much @Arturock , I should've known.

@artrz
Copy link

artrz commented Jul 20, 2018

You're welcome!

@blunket
Copy link
Contributor Author

blunket commented Jul 24, 2018

Hey @PowerKiKi , just wondering if you think this looks good or if you'd like any changes. 🙂 Also wondering, whenever you merge this, will you possibly be making a new release shortly after on composer? Thanks 😄

@PowerKiKi PowerKiKi closed this in adf95bc Aug 5, 2018
@PowerKiKi
Copy link
Member

merged, thanks ! I'll release a version soon

Dfred pushed a commit to Dfred/PhpSpreadsheet that referenced this pull request Nov 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants