-
Notifications
You must be signed in to change notification settings - Fork 8
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
alpha Objective 2: strict-by-default #153
Comments
I think we can target all strict: 'strict-vars-by-default', 'strict-subs-by-default', 'strict-refs-by-default' |
Odds are strict-vars-by-default is 95% of the work |
and the other 5% for |
Here is what I think a cleaner commit to enable strict by default |
We are now ready to begin work on Objective 2, strict-by-default. Commit 800f84a in the I have created a new branch,
@atoomic, when you are ready, could you start the work on this branch off by merging what you have been doing privately to achieve the first bullet-point above. Please let me know when you do so, since at that point I will want to run test suite to get a baseline for what we have to do there. Once we get that baseline, I will then merge in what I have been doing locally to get the test suite into shape for strict-by-default. Thank you very much. |
The first point Right now you cannot compile perl without fixing the strictness issues in multiple locations.
Any other ideas? |
Thanks for those commits. I've reviewed the diff between a08a7c1 and the starting point of the branch (800f84a or tag: alpha-01). I have several comments. First, while What comes after
Now, locally I believe I have all But to do that we should first revert the changes you've made already in these test files:
The changes in Constant.t are premature. Getting Second, once we get That would imply that we would "fix" modules under That in turn would imply that we should revert most or all of the changes in those 4 directories until the point where
The only thing where I'm unsure whether we should revert or not is changes like these:
But, here again, I think if go through what |
Follow-up to previous comment. @atoomic, I looked at these one commit at a time and made recommendations inline.
Throw out all of this except possibly the change to regen/regen_lib.pl
Looks good.
Throw out changes to cpan/ExtUtils-Constant/t/Constant.t, t/comp/require.t, t/comp/use.t.
Looks good (though behavior of
Remove changes to cpan/DB_File/DB_File_BS and ext/XS-APItest/APItest_BS. We'll add them back in when
Looks good, subject to proviso that expected behavior for
For now, let's back out these changes and start adding them back in as we work our way through So, once again, we have to do some backtracking. But I think that if we do the above, then work on getting Thank you very much. |
I've been testing out my suggestions in the previous comment ... but experiencing problems. Re-examining. |
@jkeenan feel free to revert any commit/files and push this directly to this working branch |
@atoomic, here's an update on what I was able to accomplish toward Objective 2 yesterday. We Can Now Run I created the This branch starts from tag The first 31 commits of these are the most important. If you squint, you can see that they include the actual changes to the core guts which you made to turn on strict-by-default. Some of your commits I applied verbatim via If we think these commits are correct, we would do a hard reset on My first hope (as suggested in one of my posts from Aug 07) was to get to I decided to dodge
At this point we can now call The next 200 or so commits constitute my work over the past several weeks in local branches to bring individual test files into strict-compliance. In most cases, one commit reflects work on one test file and (through the miracle of Some of these 200+ test files are not PASSing or have tests TODO-ed that are not TODO-ed in the Perl 5 test suite. I tried to indicate that in commit messages. This series of commits terminates with:
At this point, I hit
That's where I left off last night. At this point the output of Test Files Having Difficulty Locating 'strict' I now want to mention the problem alluded to above. In certain test files in the "early" directories (e.g.,
These failures are occurring during both Thank you very much. |
And now updated to Sun Aug 09.
And this is still my plan. We'll need to discuss before proceeding.
And now we're down from 42422 lines to 23480 lines.
Today to get some tests passing I put a kludge into t/test.pl for its handling of @inc. However, I now think that was not a good idea and that what I need to do tomorrow is to rewrite t/test.pl so that it is strict-by-default. Once we resolve the branch situation, here are some files that are failing that you might want to take a look at:
Thank you very much. |
Thanks James for the update, your branch seems ok, I would like to consider two changes as part of this branch.
|
On 8/10/20 11:05 AM, Nicolas R. wrote:
Thanks James for the update, your branch seems ok, I would like to
consider thew two changes as part of this branch.
I know they are not strict by themselves but they are follow up fixes to
the previous state.
1. restore behavior for |use 5.x|, block |use 6.*| and new behavior for
|use v7|
* c1eb819
<c1eb819>
Restore 'use 5.x' behavior as it behaves in Perl 5
The above sounds like something that should be done *before* embarking
on Objective 2.
Could you write this up as a separate ticket, calling it, say,
"Objective 1a: Modify behavior for certain 'use VERSION' statements"?
I would then recommend forking a new branch from tag alpha-01 and
working through the implications of the commit above. (Hint: This
might include work on t/comp/use.t.)
We'd then both test it and, when okay, merge it back into alpha, tag it
something like 'alpha-01a', and then re-fork a branch for Objective 2
for it.
2. do not enforce strict on oneliners when using |-e| or |-E|
* e9f6ca8
<e9f6ca8>
set minus_e correctly and disable strict for -e and -E
* ae66b97
<ae66b97>
do not promote strict on -e
This sounds like something we could include in Objective 2 once we've
completed Objective 1a above.
Thank you very much.
Jim Keenan
|
I created #187 to track these changes |
Progress report: I've been tracking our progress via two somewhat crude but easily obtainable data points: the number of lines in the output from By way of contrast, in Perl 5 blead When we began work on Objective 2, we got 42422 lines in At a point on Aug 09 where I started to note both the line count and the count of failures, we were at: 24543 lines As of earlier today, we're at: 16421 Thank you very much. |
Following merge of the branch for Objective 1a into alpha, this branch has been rebased on alpha. |
I have just tagged commit That means that 'MC-1' means the first Merge Candidate for the current working branch. 'alpha-02' refers to Objective 2. We should treat the When we are satisfied with the results we will merge the branch into Thank you very much. |
Smoke-testing has indeed revealed bugs which are being tracked in these tickets: As we make progress we'll make commits to the Thank you very much. |
Overnight we got a spurious smoke-testing failure due to a race condition in version 2.14 of ExtUtils-Install. I upgraded our branch to use CPAN release 2.16, then brought all files in that directory into strict-compliance. I then applied tag Thank you very much. |
Tonight I reviewed the status of our simulation branch in the Perl5 repository: http://perl.develop-help.com/?b=smoke-me%2Fjkeenan%2Fcumberland-blues. All test failures are there are either intermittent and covered by issues in this repository or are failing on the same test smokers for Perl 5 blead. I then rebased the I then tagged the HEAD of This complete our work on Objective 2, "strict-by-default". Thank you very much. |
Please rebase any branches that were forked from |
Objective: Implement 'strict-by-default'.
Modify the guts such that strictures are now on by default. The statement
use strict;
should become a "no-op" except where a particular file has previously called some variant ofno strict;
.No other guts changes except those needed to implement this objective.
Note: If the lead developer feels it would be better to sub-divide this objective into three sub-objectives -- 'strict-vars-by-default', 'strict-subs-by-default', 'strict-refs-by-default' -- that's cool. We'll just need to create tickets for that.
Initially, there will be many files in the test suite and many dual-life modules that will fail to compile once we are strict-by-default. If we were being very purist, the only revisions we would make to such files would be those that get the file to compile and, in the case of a file in the test suite, to get the test to run GREEN.
However, many files in the test suite have already been brought into compliance with both strict-by-default, warnings-by-default and other, later objectives. We can make use of that prior art by appropriately cherry-picking commits to such files from earlier branches.
Acceptance Criteria: The changes needed to have Perl be strict-by-default are clearly available from the commit history and documented in the commit log. The Perl test suite runs GREEN in all major configurations and on all major operating systems for which we can test.
The text was updated successfully, but these errors were encountered: