Thanks for the reviews. I have made the suggested changes.

The test failures on Travis seem to be related to your changes.

Thanks. I fixed my code/tests so they are passing again. Travis is now failing for reasons that appear to be unrelated to this PR.

@jdufresne is your current branch up to date with 4.4?

The parent commit isd3a7be8 (a few new commits landed minutes ago).

Looks like the 4.4 branch is also failing with the same errors as of today.

The test failures have been caused by Twig. The issue should be fixed withtwigphp/Twig@debc5c3

Thanks for the review! I've followed the linked example by dropping the argument and using the shim.

I'm divided about this proposal. Theprefix is needed because this method returns an absolute path to a file, so it's not trivial to prefix only the file name.

But, adding a suffix should always be simple because it's safe to append a string to the value returned by this function. So, I'd probably close this as "won't fix".

However, another option would be to change the current "prefix" option to be a "pattern" option with only one placeholder called{filename}, so you can do this:

// Before$fs->tempnam('/tmp','prefix_');$fs->tempnam('/tmp','prefix_','.png');// After$fs->tempnam('/tmp','prefix_{filename}');$fs->tempnam('/tmp','prefix_{filename}.png');

The prefix is needed because this method returns an absolute path to a file, so it's not trivial to prefix only the file name.

The prefix is actually a required parameter in the nativetempnam function which this method uses, so it's not just about the path.

adding a suffix should always be simple because it's safe to append a string to the value returned by this function

The problem here is that you will then manually need to create the file and remove the original file (as the method creates the file as well and doesn't just return a path).

So going from

$file = $fs->tempnam('/tmp', 'prefix_', '.png');

to this

$file = $fs->tempnam('/tmp', 'prefix_');$fs->remove($file); // remove created file$file .= '.png';$fs->touch($file); // create new file

@pierredup you explained it well. I think I'm now a bit more 👍 about this proposal.

I've made the suggested change from@stof and dropped the deprecation warning so thatDebugClassLoader handles it.

Thanks. I understand now. I have applied the suggested patch in the latest revision.

fabbot is suggesting that I should remove$suffix from the docblock. Is this expected? Should I remove it? Or should I alter it a specific way to appease the bot?

Should I remove it? Or should I alter it a specific way to appease the bot?

I would say its a false/positive

Thanks. At this stage is there anything I should do to move this PR forward?

Thank you@jdufresne.