Skip to content

Conversation

manwar
Copy link
Contributor

@manwar manwar commented Aug 9, 2017

Hi,

Please review the PR.

Many Thanks.
Best Regards,
Mohammad S Anwar

@oalders
Copy link
Member

oalders commented Aug 9, 2017

Hi @manwar. Thanks for this!

Should we add Test::FailWarnings to the test suite to ensure that we're not introducing any new warnings?

@manwar
Copy link
Contributor Author

manwar commented Aug 10, 2017

Hi @oalders

That's brilliant idea and good catch.

Best Regards,
Mohammad S Anwar

@oalders
Copy link
Member

oalders commented Dec 3, 2018

@manwar I apologize for losing track of this PR. Would you be able to resolve the conflicts and force-push your commits again?

@manwar
Copy link
Contributor Author

manwar commented Dec 3, 2018

Sure in a moment

@manwar manwar force-pushed the add-warnings-pragma branch from 64bcaa9 to 0ee372e Compare December 3, 2018 15:58
@@ -1,6 +1,8 @@
package HTTP::Cookies::Microsoft;

use strict;
use warnings;
use vars qw(@ISA);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why's the legacy vars pragma being introduced? @ISA is declared correctly with our a few lines down. Though something like parent might be better still it's out of the scope of this PR.

@oalders
Copy link
Member

oalders commented Sep 9, 2022

@manwar is the feedback something you'd like to address? I promise to keep track of the PR this time. 😢

@manwar
Copy link
Contributor Author

manwar commented Sep 9, 2022

@manwar is the feedback something you'd like to address? I promise to keep track of the PR this time. 😢

Sure, I will look into this weekend, promise.

@manwar manwar force-pushed the add-warnings-pragma branch from 0ee372e to 5b1a710 Compare September 11, 2022 00:45
@manwar
Copy link
Contributor Author

manwar commented Sep 11, 2022

This is all messed up. I would rather create fresh pull request. Please ignore this pull request.

@manwar manwar closed this Sep 11, 2022
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.

3 participants