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

inspect warnings written into code #176

Open
sunnysideup opened this issue Jun 17, 2019 · 3 comments
Open

inspect warnings written into code #176

sunnysideup opened this issue Jun 17, 2019 · 3 comments

Comments

@sunnysideup
Copy link

sunnysideup commented Jun 17, 2019

I was wondering if it would be possible to have the inspect warnings written into the code...

e.g.

BEFORE

Session::get('foo', 'bar');

AFTER

//@todo: upgrade me ... Session can not be called using static methods in SS4, see bla.foobar.com
Session::get('foo', 'bar');

The reason for this is that they are easily missed when doing the upgrade on the command line and if you are working on the project over several days then you want an easy way to find the "todo" things.

@sunnysideup
Copy link
Author

sunnysideup commented Jun 17, 2019

I am happy to write this code for the upgrade tool IF someone can me a few pointers on how to do this.

Alternatively, someone could perhaps explain how to get the warnings data in a parseable format so that we could run the inspect command and add the comments ourselves using the outputted data.

e.g.

[PATH], [LINE], [MESSAGE], [URL]
[PATH], [LINE], [MESSAGE], [URL]
[PATH], [LINE], [MESSAGE], [URL]
'app/src/Hello.php', 23, 'Session can not be called using static methods in SS4', 'bla.foobar.com'

@maxime-rainville
Copy link
Contributor

I'm not sure how much value this would provide. The initial logic wasn't written with this idea in mind.

If I wanted to do this, I would probably try to add it to CodeChangeSet, this is where we track the changed code and the warnings. That's what uses to compiled the diff we show users.

If you want to have a go at creating a PR for this, maybe do a very rough proof of concept first.

@sunnysideup
Copy link
Author

sunnysideup commented Jul 13, 2019

In my mind it adds a ton of value, for the following reasons:

  • in our current system, as soon as you make one change to a file, the line reference to further warnings are lost.
  • because you can search through the code for "UPGRADE REQUIRED", or whatever you want to use, and then you can see the whole code immediately, in all its detail. We use this system in https://github.com/sunnysideup/silverstripe-upgrade_to_silverstripe_4 and it is one of the best parts. It adds a neat comment above any line that needs changing, with the suggested change and explanation.
  • because saving the warnings to the actual code means they are never "overlooked" and you delete the warning from the code once completed.

Would it be possible to get the output like this:
[PATH], [LINE], [MESSAGE], [URL]
'app/src/Hello.php', 23, 'Session can not be called using static methods in SS4', 'bla.foobar.com'

or perhaps have the option to save this into a file?

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

No branches or pull requests

2 participants