Problem/Motivation
- There are unnecessary lines of code.
- They may add uneeded function calls or overhead.
- Removing the unused variables reduces noise in files and makes less code that needs to be maintained.
- These issues are nice ones for people to learn the core process. (After doing a few, and reviewing a few, find other issues in other topics by looking for other novice issues: https://drupal.org/project/issues/search/drupal?status%5B%5D=Open&versio... or other metas. See irc factoid: metas? Which links to the methods meta #2016679: Expand Entity Type interfaces to provide methods, protect the properties and properties rename meta #2052421: [META] Rename Views properties to core standards Another option is to jump in #drupal-contribute and ask for someone to recommend an issue to work on, or attend core mentoring office hours to find issues matched to your interest and skills.)
Proposed resolution
Remove the lines of code that declare the unused variables.
Is there a way to automatically detect? No. See #12-#14.
Remaining tasks
- (done) Identify each file that has an unused variable.
- (done) Make one issue per file.
- Also, write down reproducable steps to identifying per file, each unused variable. Might require IDE.
- (done) Create the issue and list the variables in that issue.
- (done) Add the file to the table.
- (done) Link the issue in the table.
- Do a few that are active or needs work.
- Review a few. See hints for reviewers.
Hints
Be careful of an include_once. It might use the variables that an IDE says is unused, but it is actually used. There are very few places in core that do that, but be careful of it.
(Small note be careful of list(). Most people can ignore this.)
Hints for reviewers
- Look at the patch with dreditor (http://dreditor.org).
- Are there unintended changes? Ask about them.
- Was trailing whitespace accidentally included (an example at #2067551-6: /core/lib/Drupal/Core/Routing/MatcherDumper.php will never roll back its transaction)? If so, instead of just pointing it out in a comment, also go ahead and make a new patch and interdiff that removes the whitespace. For instructions on creating an interdiff, see https://drupal.org/documentation/git/interdiff | Microbranching workflow: http://xjm.drupalgardens.com/blog/interdiffs-how-make-them-and-why-they-...
- Apply the patch, then
- If it does not apply:
- If you have time, reroll it. (reroll instructions: http://drupal.org/patch/reroll) Interdiffs usually do not make sense with rerolls, but you can add some info like saying if it was an automatic merge, if there were conflicts, and if you found the issue/commit that caused it to not apply, mention that. If you do not have time to reroll it, mark the status needs work, and add the Needs reroll tag.
- If the patch cannot be applied because it was already fixed by other patch (the line/lines to fix dissapeared), find the issue where that patch was posted and reference it. Then, you can mark it as "Closed (won't fix)" . Example: #2080539: Remove Unused local variable $langcode from /core/modules/forum/lib/Drupal/forum/Tests/ForumBlockTest.php
- If it did apply, recheck (by opening the file in an IDE like phpstorm) if any new unused variables are revealed by the removal of other unused variables.
- If it does not apply:
- Look at the code near the var. If you think the unused var.. actually needed to be used, you may have spotted a flaw that needs to be fixed. (For example: #2002708-7: Remove unused local variables from core/includes/common.inc and #2067551-5: /core/lib/Drupal/Core/Routing/MatcherDumper.php will never roll back its transaction.)
- Summarize what you checked, how you checked them, and your findings (instead of just saying "looks good"). For example #2080367-2: Remove Unused local variable $storageFactory from core/tests/Drupal/Tests/Component/PhpStorage/MTimeProtectedFileStorageTest.php.
- Also see https://drupal.org/contributor-tasks/review for more info.
From #10 When making an issue, put the number of the new issue between the # and @ and it will make a great link. (the @ shows who it is assigned to, if it is assigned). Remove the create link when putting the issue number in.
Anyone can use the create link to make the issue and then edit this issue summary.
User interface changes
No ui changes.
API changes
No api changes.
Comment | File | Size | Author |
---|---|---|---|
#66 | unused.zip | 19.36 KB | andypost |
#61 | 2002650-61.patch | 499 bytes | andypost |
Comments
Comment #0.0
YesCT CreditAttribution: YesCT commentedUpdated issue summary.
Comment #0.1
YesCT CreditAttribution: YesCT commentedinit
Comment #1
nod_Maybe to give an idea, here is what phpstorm tells me: 626 unused variables in our code.
Attached the html output listing files and variables. The xml file has line number.
Comment #1.0
nod_noting something.
Comment #2
amateescu CreditAttribution: amateescu commentedDoing an issue per file sounds a bit too much, I'd propose at most one issue per folder in core/ (includes, lib, modules/*, tests, etc..). I started with the one for core/includes/* and added it to the issue summary.
Comment #2.0
nod_Added #2002704: Remove unused local variables from files in core/includes to the list.
Comment #3
nod_uploading the xml separately, so that ppl don't need to mess with an archive to see what's up.
( edit ) I would agree. We did one issue per file for the same thing in JS and it just takes forever and there are a lot more PHP files. When it's during a sprint it's a bit different though, that could work.
Comment #4
YesCT CreditAttribution: YesCT commented@alexpott asked for these to be one issue per file.
Some of them might need more discussion that are part of settings includes or list().
These are good first issues for new contributors, so while it might seem like a lot of work the pay back will be great.
Comment #4.0
YesCT CreditAttribution: YesCT commentedUpdated issue summary.
Comment #4.1
YesCT CreditAttribution: YesCT commentedUpdated issue summary.
Comment #4.2
kerasai CreditAttribution: kerasai commentedUpdated issue summary.
Comment #4.3
kerasai CreditAttribution: kerasai commentedUpdated issue summary.
Comment #4.4
kerasai CreditAttribution: kerasai commentedUpdated issue summary.
Comment #4.5
kerasai CreditAttribution: kerasai commentedUpdated issue summary.
Comment #4.6
kerasai CreditAttribution: kerasai commentedUpdated issue summary.
Comment #4.7
kerasai CreditAttribution: kerasai commentedUpdated issue summary.
Comment #4.8
kerasai CreditAttribution: kerasai commented2002732
Comment #4.9
kerasai CreditAttribution: kerasai commentedUpdated issue summary.
Comment #6
nod_Is there an issue for unused "use" uses? if not here is the list of the 936 of them.
Comment #7
tim.plunkettBeware that list, it includes Annotation classes which many IDEs don't count as being used. Removing them will break everything.
Comment #8
alexpottLet's not over-egg the performance impact here... each unused variable / line of code will consume memory and cpu time but it's minuscule compared to other factors.
One is very important is that each line of code we have in the codebase needs to be maintained therefore removing cruft is a good thing. It's part of making Drupal a nice place to code... less thinking hmmm... that looks unused... but it is really?
Comment #8.0
alexpottUpdate triggered
Comment #9
YesCT CreditAttribution: YesCT commentedI need to figure out a way to parse the xml, or get it in a different format. We should expand the table to list more files so the issues can be made.
Comment #10
nod_Updated export and php script to generate the table and link to create issues.
Remove all underscore from the files below and it should work.
Comment #10.0
nod_moving the hints, as the table grows, people will not see them
Comment #10.1
YesCT CreditAttribution: YesCT commentedjust adding the whole table. need to check if issues already in the summary are there and remove them.
Comment #10.2
YesCT CreditAttribution: YesCT commentedusing [#@] instead of [#} to see who it is assigned to
Comment #10.3
tstoecklerUpdated issue summary. Added 2 new issues.
Comment #10.4
tstoecklerUpdated issue summary. Removed stale rows from the second table.
Comment #10.5
tstoecklerUpdated issue summary for #2056445
Comment #10.6
jan.stoecklerUpdated issue summary.
Comment #10.7
jan.stoecklerUpdated issue summary.
Comment #10.8
jan.stoecklermodified the listing, specifically concerning /modules/testswarm/testswarm.pages.inc
Comment #10.9
jan.stoeckleradded reference to new issue #2057019
Comment #10.10
legolasbo- Created an issue for the removal of variables in /core/modules/user/user.module
- Removed table row for /core/modules/user/user.admin.inc because that file has no unused variables any more.
Comment #10.11
legolasbocreated an issue for core/modules/block/block.module
Comment #10.12
legolasboMoved my issues to the issue table.
Comment #11
kerasai CreditAttribution: kerasai commented@nod_ this is awesome, thanks.
All clear to start knocking some of these out?
Comment #11.0
kerasai CreditAttribution: kerasai commentedFixed a copy paste error and added issue for /core/modules/views/views.module
Comment #11.1
tstoecklerUpdated issue summary.
Comment #11.2
legolasboCreated some issues and altered the tables accordingly
Comment #11.3
legolasboCreated /core/lib/Drupal/Core/Database/ related issues and altered the tables accordingly
Comment #11.4
manu4543 CreditAttribution: manu4543 commentedUpdated issue summary to include the new created issue.
Comment #11.5
manu4543 CreditAttribution: manu4543 commentedUpdating issue summary for adding issue https://drupal.org/node/2061387
Comment #11.6
manu4543 CreditAttribution: manu4543 commentedUpdated issue summary for adding issue https://drupal.org/node/2061397
Comment #11.7
duozerskCreated one sub-issue
Comment #11.8
duozerskAdded new sub-issue
Comment #11.9
duozerskAdded 2 sub-issues
Comment #11.10
duozerskAdded 2 more sub-issues
Comment #11.11
duozerskCombined 3 lines into one.
Comment #11.12
duozerskAdded 3 more sub-issues
Comment #11.13
duozerskAdded 4 sub-issues.
Comment #11.14
sergeypavlenko CreditAttribution: sergeypavlenko commentedAssign /core/lib/Drupal/Core/Routing/RouteProvider.php
Comment #11.15
sergeypavlenko CreditAttribution: sergeypavlenko commentedUpdated issue summary.
Comment #11.16
sergeypavlenko CreditAttribution: sergeypavlenko commentedUpdated issue summary.
Comment #11.17
sergeypavlenko CreditAttribution: sergeypavlenko commentedUpdated issue summary.
Comment #11.18
sergeypavlenko CreditAttribution: sergeypavlenko commentedUpdated issue summary.
Comment #11.19
sergeypavlenko CreditAttribution: sergeypavlenko commentedUpdated issue summary.
Comment #11.20
sergeypavlenko CreditAttribution: sergeypavlenko commentedUpdated issue summary.
Comment #11.21
sergeypavlenko CreditAttribution: sergeypavlenko commentedUpdated issue summary.
Comment #11.22
jlindsey15 CreditAttribution: jlindsey15 commentedAdded NodeSaveTest issue
Comment #11.23
jlindsey15 CreditAttribution: jlindsey15 commentedRemoved "create" button for nodesavetest issue
Comment #11.24
jlindsey15 CreditAttribution: jlindsey15 commentedAdded issue
Comment #11.25
sergeypavlenko CreditAttribution: sergeypavlenko commentedUpdated issue summary.
Comment #11.26
sergeypavlenko CreditAttribution: sergeypavlenko commentedUpdated issue summary.
Comment #11.27
jlindsey15 CreditAttribution: jlindsey15 commentedAdded child issue
Comment #11.28
jlindsey15 CreditAttribution: jlindsey15 commentedAdded child issue
Comment #11.29
jlindsey15 CreditAttribution: jlindsey15 commentedAdded child issue.
Comment #11.30
jlindsey15 CreditAttribution: jlindsey15 commentedAdded child issue.
Comment #11.31
legolasboCleaned up the second table by moving active issues to the first one
Comment #11.32
legolasboCreated some issues
Comment #11.33
legolasboCreated some more issues
Comment #11.34
mrsinguyen CreditAttribution: mrsinguyen commentedAdded the #2079857
Comment #11.35
mrsinguyen CreditAttribution: mrsinguyen commentedAdded #2079863
Comment #11.36
mrsinguyen CreditAttribution: mrsinguyen commentedAdded #2079881
Comment #11.37
mrsinguyen CreditAttribution: mrsinguyen commentedAdded #2079887
Comment #11.38
mrsinguyen CreditAttribution: mrsinguyen commentedAdded #2079891
Comment #11.39
mrsinguyen CreditAttribution: mrsinguyen commentedUpdated issue summary.
Comment #11.40
mrsinguyen CreditAttribution: mrsinguyen commentedUpdated issue summary.
Comment #11.41
mrsinguyen CreditAttribution: mrsinguyen commentedUpdated issue summary.
Comment #11.42
mrsinguyen CreditAttribution: mrsinguyen commentedUpdated issue summary.
Comment #11.43
mrsinguyen CreditAttribution: mrsinguyen commentedUpdated issue summary.
Comment #11.44
mrsinguyen CreditAttribution: mrsinguyen commentedUpdated issue summary.
Comment #11.45
mrsinguyen CreditAttribution: mrsinguyen commentedUpdated issue summary.
Comment #11.46
mrsinguyen CreditAttribution: mrsinguyen commentedUpdated issue summary.
Comment #11.47
mrsinguyen CreditAttribution: mrsinguyen commentedUpdated issue summary.
Comment #11.48
mrsinguyen CreditAttribution: mrsinguyen commentedUpdated issue summary.
Comment #11.49
mrsinguyen CreditAttribution: mrsinguyen commentedUpdated issue summary.
Comment #11.50
mrsinguyen CreditAttribution: mrsinguyen commentedUpdated issue summary.
Comment #11.51
mrsinguyen CreditAttribution: mrsinguyen commentedUpdated issue summary.
Comment #11.52
mrsinguyen CreditAttribution: mrsinguyen commentedUpdated issue summary.
Comment #11.53
mrsinguyen CreditAttribution: mrsinguyen commentedUpdated issue summary.
Comment #11.54
mrsinguyen CreditAttribution: mrsinguyen commentedUpdated issue summary.
Comment #11.55
mrsinguyen CreditAttribution: mrsinguyen commentedUpdated issue summary.
Comment #11.56
mrsinguyen CreditAttribution: mrsinguyen commentedUpdated issue summary.
Comment #11.57
mrsinguyen CreditAttribution: mrsinguyen commentedUpdated issue summary.
Comment #11.58
mrsinguyen CreditAttribution: mrsinguyen commentedUpdated issue summary.
Comment #11.59
mrsinguyen CreditAttribution: mrsinguyen commentedUpdated issue summary.
Comment #12
webchickI'm curious about something. Is it possible to write some kind of automated test that will catch these as they're introduced? One would assume PHPStorm is hooking into some kind of PHP Reflection library or similar, but not sure. I am just concerned that we're going through all of this effort that's going to eventually be eroded away without some means of people without PHPStorm ensuring the code base stays sane.
Comment #13
dawehnerThere are some places in core which let's IDEs think that variables are unused even they are not (sadly I don't remember the example).
http://phpmd.org/ seems to be what you want to do.
Comment #14
alexpottIn an ideal world we would be running phpmd as part of testbot. But it is going to take a while to get there.
Comment #14.0
alexpottUpdated issue summary.
Comment #14.1
YesCT CreditAttribution: YesCT commentedadded info about how to move on to other types of issues.
Comment #15
YesCT CreditAttribution: YesCT commentedWondering how to find other issues to work on?
These issues are nice ones for people to learn the core process. (After doing a few, and reviewing a few, find other issues in other topics by looking for other novice issues: https://drupal.org/project/issues/search/drupal?status%5B%5D=Open&versio... or other metas. See irc factoid: metas? Which links to the methods meta #2016679: Expand Entity Type interfaces to provide methods, protect the properties and properties rename meta #2052421: [META] Rename Views properties to core standards Another option is to jump in #drupal-contribute and ask for someone to recommend an issue to work on, or attend core mentoring office hours to find issues matched to your interest and skills.)
(updated issue summary)
Comment #15.0
YesCT CreditAttribution: YesCT commentedadded html ending tag
Comment #15.1
YesCT CreditAttribution: YesCT commentedupdated remaining tasks.
Comment #16
andypostWe should find better way to work on this kind of patches.
From IRC:
Comment #17
YesCT CreditAttribution: YesCT commentedSome guidance for directing people to other issues has been added... maybe some more, like 3 of these a day please? Along with the "how to find other issues" would alleviate some of the testbot strain.
The bot is back to 0 queued now. Did @jthorson end up spinning up the extra 20 testbots to clear it?
Hm. If we use this in Prague on the Get Involved Day... we will run into the same issue. Even if we have 50 people only do one of these a piece. Well no matter what people work on actually. It's just that they could do one of these *and* another issue during the day. I'd better talk to @jthorson.
Comment #18
mrsinguyen CreditAttribution: mrsinguyen commentedit's my wrong when create many issues in yesterday. I think we need combine many issue for one patch.
Comment #18.0
mrsinguyen CreditAttribution: mrsinguyen commentedsmall grammar fix
Comment #18.1
YesCT CreditAttribution: YesCT commentedadded more hints and examples for reviewers.
Comment #18.2
YesCT CreditAttribution: YesCT commenteduh, i can write html. fixing.
Comment #18.3
YesCT CreditAttribution: YesCT commentedmore html fix
Comment #18.4
YesCT CreditAttribution: YesCT commentedadded hint about checking if it applies and reroll.
Comment #18.5
YesCT CreditAttribution: YesCT commentedanother hint to say what was checked.
Comment #18.6
grisendo CreditAttribution: grisendo commentedUpdated issue summary.
Comment #18.7
grisendo CreditAttribution: grisendo commentedUpdated issue summary.
Comment #18.8
grisendo CreditAttribution: grisendo commentedChanged link to the issue with tokens
Comment #19
vijaycs85Comment #20
vijaycs85Cleaning up sub-tasks.
Comment #21
vijaycs85Cleaning up sub-tasks.
Comment #22
vijaycs85More update to sub-tasks
Comment #23
webchickAs much as I love initiatives like this to help get new people involved in core, these issues are definitely taking their toll. Over 50 of the 158 RTBC issues coming back from Thanksgiving were these issues. So tonight I literally spent
45 hours (and counting) doing just about nothing else but reviewing/committing/needs working these. That's45 hours I didn't spend reviewing major/critical issues that help push D8 further towards release. :( And I only get about one of these45-hour uninterrupted chunks per week.This work is very valuable; a few of these have managed to catch some real actual bugs. But is it possible maybe to start combine some of these patches into bigger, less granular ones (per-file is just a bit nuts) so that the needs review/needs work/needs review/RTBC/commit cycle is more justified?
Comment #24
xjmPlease change the structure of this so that patches are rolled at a minimum on a per-module basis. Per file is unnecessarily granular.
Also, if you have already done 2 or 3 of these tasks, please leave the remainder for others to complete.
Comment #25
xjmAlso, please file all the sub-tasks as minor so that they can be sorted from normal issues.
Comment #27
xjmMoved the fixed rows down so we can see what issues are still listed.
Comment #28
xjmI reopened a couple that had actual bugs associated with them and retitled them to reflect their actual scope. When you discover a bug in the process of rolling a patch to remove several from a module, it makes sense to spin off a normal, targeted bug report. Thanks everyone!
Comment #29
webchickTagging.
Comment #30
aspilicious CreditAttribution: aspilicious commentedComment #31
aspilicious CreditAttribution: aspilicious commentedComment #32
YesCT CreditAttribution: YesCT commentedupdating a bit of what issue is doing what.
Comment #33
YesCT CreditAttribution: YesCT commentedupdating a bit of what issue is doing what.
Comment #34
YesCT CreditAttribution: YesCT commentedupdating what core/tests is covering
Comment #35
YesCT CreditAttribution: YesCT commentedupdating what core/tests is covering
Comment #36
lokapujyaWould be great to get this reviewed before I need to reroll again: #2080343: Remove Unused local variables from system module
Comment #37
lokapujyaWould be great to get this reviewed before I need to reroll again: #2080343: Remove Unused local variables from system module
Comment #38
jmarkel CreditAttribution: jmarkel as a volunteer commentedClosing this issue since it appears that none of the included issues appear to be open. If there are any remianing that are indeed still open they can be addressed as one-offs on their own.
Comment #39
andypostAt least 2 issues open
#2072597: Remove Unused local variables from tests in the Views module
#2081137: Remove unused local variables from /core/includes/bootstrap.inc
And I'm sure there should be more filed, and we need to run inspections again!
Comment #40
xjmThanks everyone for your work on these issues!
In general, I'd recommend the following for managing these issues:
Removing the table of issues from the summary since doing one patch per file is not desirable. Also toning down some of the claims about performance in the issue summary since in most cases the performance impact is immeasurably small on a typical request.
Comment #53
alexpottIn the intervening years things have moved on and we can automated this.
In order to fix core coding standards in a maintainable way, all our coding standards issues should be done on a per-rule basis across all of core, rather than fixing standards in individual modules or files. We should also separate fixes where we need to write new documentation from fixes where we need to correct existing standards. This all should be done as part of #2571965: [meta] Fix PHP coding standards in core. A good place to for unused variables is #3106216: Remove unused variables from core.
For background information on why we usually will not commit coding standards fixes that aren't scoped in that way, see the core issue scope guidelines, especially the note about coding standards cleanups. That document also includes numerous suggestions for scoping issues including documentation coding standards cleanups.
Contributing to the overall plan above will help ensure that your fixes for core's coding standards remain in core the long term.
Comment #54
alexpottIn discussion with xim, catch and larowlan, my earlier comment is incorrect. We should handle each unused variable on its own merit and do the work to work out why it is not used.
This can point to broken code or incomplete testing see #3157369: Use unused variable $filters from DateTimeSchemaTest for example. A useful tool for this is
git log -S “SOME TEXT”
which will search git commits for matching text to find out when the variable might have become unused. Without doing the work to show why the variable is unused the patch will not be committed. Alsogit blame
can be useful as well.So let's leave this open to track the downstream work. Note it is still important to automate this so we can prevent future regressions and catch this prior to the fact rather than after.
Comment #55
Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commentedI just NW'd 3 issues based on what I understand from #54:
#3164496: Unused variable $langcode in content_moderation module, ModerationLocaleTest.php
#3164498: Unused variable $entity_type_ids in content_moderation module, content_moderation.module
#3164500: Unused variable $id in content_moderation module, Permissions.php
I understand less variables to parse is easier to read, but the benefit is seriously marginal in situations like the following:
Is it more appropriate to just close issues like this, or once this becomes part of the official standard, will these kind of changes be required anyway?
Comment #56
longwaveShould we try and deal with all unused key variables in foreach loops in a single issue, and then handle all the other cases individually?
Comment #57
catchI've been marking foreach loop changes as 'by design', since it's often more readable to specify what the key is whether it gets used or not.
Comment #58
jungle+1 to #56, foreach should not be an exception, especially, if we are going to apply a phpcs sniff/rule for unused local variables stickly in the future.
Comment #59
jungleComment #60
jungleThe rule/sniff is
DrupalPractice.CodeAnalysis.VariableAnalysis.UnusedVariable
, to identify unused variables, we can enable this rule and runcomposer run phpcs
locally.Comment #61
andypostThere's just 4 child issues left, so makes sense to add this rule to core and count remains
Also there's https://github.com/sirbrillig/phpcs-variable-analysis
Meantime the patch to try with core
Comment #62
longwave> foreach should not be an exception, especially, if we are going to apply a phpcs sniff/rule for unused local variables stickly in the future.
We could modify/extend a sniff to ignore keys in foreach loops, if policy is that they are allowed to be unused?
Comment #63
jungleRe #62, it depends on commiters' favor. Per #57, I think we can go for it :)
BTW, the current rule can not detect unused $value in foreach, for example:
Comment #64
catchI think we probably need a coding standards issue to make it official, but my feeling is that unused variables in foreach is a style decision - as opposed to other unused variables which are always going to be either cruft or bugs.
Comment #65
jungle> allowUnusedForeachVariables (bool, default true): if set to true, unused values from the key => value syntax in a foreach loop will never be marked as unused.
From https://github.com/sirbrillig/phpcs-variable-analysis which shared by @andypost in #61
Comment #66
andypostSniffer in #62 shows 260 usages https://www.drupal.org/pift-ci-job/1811507
Also attaching phpstorm results in "core" (147 places)
Comment #67
jungleFor unused variables in Tests, I think they could be removed in one go, it's unnecessary digging into the history. Can we do it in one seperate issue?
Comment #68
longwaveDisagree with #67, an unused variable in a test might be a refactor gone wrong that wasn't spotted at the time, and we are not testing something we think we are testing.
Comment #69
jungleGood point, thanks @longwave!
Comment #70
andypostRe #65 this library listed after upgrade of drupal/coder 8.3.10
Comment #75
longwaveCan this be marked as fixed now, given that we completed #3106216: Remove unused variables from core and also we have PHPStan in core?
Comment #77
andypostNew child added #3320483: Remove unused variable $pos in system.install
Comment #78
quietone CreditAttribution: quietone at PreviousNext commentedI agree with longwave that this can be closed now. Any remaining instances of an unused variable can be worked on as a child of the Meta issue that is coordinating the remaining work of implementing the DrupalPractice sniff. #3348079: Fix DrupalPractice.CodeAnalysis.VariableAnalysis
Congratulations to everyone for managing these changes!
Thanks!