search

PHPOffice / PhpSpreadsheet

A pure PHP library for reading and writing spreadsheet files
https://phpspreadsheet.readthedocs.io
MIT License
13.34k stars 3.46k forks source link

Raising Scrutinizer index #240

Closed SenseException closed 6 years ago

SenseException commented 7 years ago

This is:

What is the expected behavior?

Having no major issues in Scrutinizer.

What is the current behavior?

Multiple issues, that are error prone.

For instance, missing properties in a class are fine, if someone writes into them first, but will result in an error, if a read operation comes first.

What are the steps to reproduce?

Those issues are already reproduced by Scrutinizer.

https://scrutinizer-ci.com/g/PHPOffice/PhpSpreadsheet/issues/develop?selectedSeverities%5B0%5D=10&orderField=path&order=asc&honorSelectedPaths=0

Which versions of PhpSpreadsheet and PHP are affected?

develop and master Branches are affected. This project need to reduce listed issues with multiple clean-up pull requests to prevent current and future issues and bugs. It needs a cleaner code handling too, why var_dump and similar debugging code shouldn't be committed into the develop branch.

This needs to be handled before new features are introduces. Also, no PRs introducing new crucial Severities should be merged without reasons.

PowerKiKi commented 7 years ago

The best would probably to submit a PR for each type of issue found by scrutinizer. Or for a single issue if the changes are too important. Would you be willing to work on that?

SenseException commented 7 years ago

I can provide some. Also, anybody is invited to pick up a few severities too.

endelwar commented 7 years ago

@PowerKiKi you should ask Scrutinizer to enable their Security Analysis, it has been very valuable on another project that I follow

PowerKiKi commented 7 years ago

@endelwar, I believe this is only available for paid account. So we won't be able to use it unfortunately...

endelwar commented 7 years ago

@PowerKiKi it's free for non-commercial open-source project! Just send them an email (support@scrutinizer-ci.com) asking for it and pointing to project's github page.

PowerKiKi commented 7 years ago

@endelwar, it has been enabled, though since we don't deal directly with user inputs, it seems it's almost pointless for us... but just in case...

endelwar commented 7 years ago

good, don't understimate the "bad guys" ;)

stale[bot] commented 6 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is still an issue for you, please try to help by debugging it further and sharing your results. Thank you for your contributions.

SenseException commented 6 years ago

I'll see that I will find some time for another Scrutinizer fix PR.

stale[bot] commented 6 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is still an issue for you, please try to help by debugging it further and sharing your results. Thank you for your contributions.

SenseException commented 6 years ago

New Scrutinizer PR added in #384.

stale[bot] commented 6 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is still an issue for you, please try to help by debugging it further and sharing your results. Thank you for your contributions.

oleibman commented 3 months ago

Scrutinizer messages have been analyzed and mostly resolved in a number of PRs over the past few years (e.g. PR #3109).