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

Prepare for symfony 4 #SymfonyConHackday2017 #125

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

Jan0707
Copy link

@Jan0707 Jan0707 commented Nov 18, 2017

Minor edits to enable installation with symfony 4

@Jan0707 Jan0707 changed the title Prepare for symfony 4 Prepare for symfony 4 #SymfonyConHackday2017 Nov 18, 2017
@ghost
Copy link

ghost commented Dec 4, 2017

@Jan0707 Please also update the service definition to use the FQCN's. 😁

@ireznik
Copy link

ireznik commented Dec 6, 2017

@Jan0707 any plans on fixing the mentioned issues in the near future?

@Jan0707
Copy link
Author

Jan0707 commented Dec 8, 2017

Sorry folks, I was travelling. I renamed the service, but added a legacy alias, so that we do not break all the installations out there ;)

@ghost
Copy link

ghost commented Dec 11, 2017

Please create a new release when this is fixed @Jan0707.

@Jan0707
Copy link
Author

Jan0707 commented Dec 12, 2017

Not really sure what to do about the memory issue here ...

@adiq
Copy link

adiq commented Jan 2, 2018

Hey, I have added some tweaks to this changes: Jan0707#1
https://travis-ci.org/adiq/ExcelBundle/builds/324158521

This fixes:

  • memory issue by injecting php ini file with memory_limit=-1
  • some build issues by correcting symfony versions
  • readme tweaks to use the new service name

Now, it all comes down to what versions of symfony/php would you want to support. From what I discovered symfony versions <3.2 have an issue when running tests due to var-dumper package not been loaded (see symfony/symfony#20201 and symfony/symfony#21491).

Maybe some compromise would be just to release v3 version as the users of older symfony versions would not benefit from this changes IMHO and it would be easier to maintain ;-)

@Jan0707
Copy link
Author

Jan0707 commented Jan 2, 2018

Could we at least manage to support 3.4 as LTS ?

@adiq
Copy link

adiq commented Jan 2, 2018

That's the question to the maintainer. Newest 3.X version is included in tests and is passing successfully

@Jan0707
Copy link
Author

Jan0707 commented Jan 4, 2018

@adiq I just merged your pr into my pr.

@Tomsgu
Copy link

Tomsgu commented Jan 19, 2018

@liuggio can you please merge this and tag a new release? Or is there any issue with the pr?

@mrljaime
Copy link

Hello people.
I taked a look into travis test and the test doesn't finish well becuase composer can't be dowloaded.
I newbie at this time contribuying to some repository but I want to make use of the bundle over symfony's 4 version.

Can I do something to run travis test again to gain that someone aprove this PR?

Cheers.
Jaime.

@adiq
Copy link

adiq commented Jan 23, 2018

@mrljaime as PhpExcel lib in this bundle is deprecated and maintainer doesn't want to support this, it's recommended to use https://github.com/roromix/SpreadsheetBundle instead. It's using new, maintained lib, is compatible with newer versions of Symfony and is similar in use to this bundle ;-)

@mrljaime
Copy link

Thank's a lot @adiq

I will use it right now.

@ghost
Copy link

ghost commented Jan 29, 2018

@liuggio Are you planning on checking out this PR or is it time that we use a different bundle?

```

- Create an object from a file:

``` php
$phpExcelObject = $this->get('phpexcel')->createPHPExcelObject('file.xls');
$phpExcelObject = $this->get('Liuggio\ExcelBundle\Factoryl')->createPHPExcelObject('file.xls');
Copy link

Choose a reason for hiding this comment

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

typo "Factoryl"

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

6 participants