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

minor syntax update #43534

Merged
merged 2 commits into from
Mar 16, 2024
Merged

minor syntax update #43534

merged 2 commits into from
Mar 16, 2024

Conversation

ArtificialOwl
Copy link
Member

@ArtificialOwl ArtificialOwl commented Feb 12, 2024

Some small minor syntax update.

I have currently no idea how we can manage this FIXME:

// FIXME: Add this for backwards compatibility, should be fixed at some point probably
if ($config === null) {
$this->config = \OC::$server->getSystemConfig();
}

It comes from a 9 years old commit e79c255
🫂 @MorrisJobke

2 solutions:

  • removing the code and the null type of $config
  • or removing the FIXME comment line.

@ArtificialOwl ArtificialOwl added the 2. developing Work in progress label Feb 12, 2024
@ArtificialOwl ArtificialOwl added this to the Nextcloud 29 milestone Feb 12, 2024
@ArtificialOwl ArtificialOwl force-pushed the enh/noid/leave-log-early-if-no-crash-reporter branch from bf68594 to 18a6b62 Compare February 12, 2024 18:17
@ArtificialOwl ArtificialOwl force-pushed the enh/noid/log-mnor-syntax-update branch 2 times, most recently from dfab91a to 2455381 Compare February 12, 2024 19:18
@ArtificialOwl ArtificialOwl force-pushed the enh/noid/leave-log-early-if-no-crash-reporter branch 4 times, most recently from bf6cb92 to 2232753 Compare February 14, 2024 15:06
@ArtificialOwl ArtificialOwl force-pushed the enh/noid/log-mnor-syntax-update branch from 2455381 to de5a1cf Compare February 14, 2024 17:44
Base automatically changed from enh/noid/leave-log-early-if-no-crash-reporter to master February 23, 2024 20:56
@ArtificialOwl ArtificialOwl force-pushed the enh/noid/log-mnor-syntax-update branch 2 times, most recently from 8117def to 9148d4c Compare February 29, 2024 14:45
@ArtificialOwl ArtificialOwl added 3. to review Waiting for reviews php Pull requests that update Php code and removed 2. developing Work in progress labels Feb 29, 2024
@MorrisJobke
Copy link
Member

  • or removing the FIXME comment line.

That's a nice one 😆

  • removing the code and the null type of $config

That makes sense, but check if some initializing code passes in null as well. I guess that was my lazy self back then. I didn't wanted to check all the callers 🙈

@ArtificialOwl
Copy link
Member Author

Hello Sir !

Hardcoded initialisation in core are fine, i'll remove your code :)

@ArtificialOwl ArtificialOwl force-pushed the enh/noid/log-mnor-syntax-update branch from 9148d4c to b502b8e Compare March 5, 2024 12:49
Copy link
Contributor

@emoral435 emoral435 left a comment

Choose a reason for hiding this comment

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

Good ol' cleaning commit, love to see this 🥳

@ArtificialOwl ArtificialOwl force-pushed the enh/noid/log-mnor-syntax-update branch 3 times, most recently from 4ece900 to 2d0a80a Compare March 11, 2024 13:49
@Altahrim Altahrim mentioned this pull request Mar 12, 2024
@ArtificialOwl ArtificialOwl force-pushed the enh/noid/log-mnor-syntax-update branch from 2d0a80a to 618aadd Compare March 12, 2024 11:19
@Altahrim Altahrim mentioned this pull request Mar 14, 2024
@Altahrim Altahrim removed the 3. to review Waiting for reviews label Mar 15, 2024
@Altahrim Altahrim added 3. to review Waiting for reviews 4. to release Ready to be released and/or waiting for tests to finish labels Mar 15, 2024
@susnux susnux removed the 3. to review Waiting for reviews label Mar 15, 2024
@skjnldsv skjnldsv merged commit e2f08d9 into master Mar 16, 2024
153 of 160 checks passed
@skjnldsv skjnldsv deleted the enh/noid/log-mnor-syntax-update branch March 16, 2024 12:21
@Altahrim Altahrim mentioned this pull request Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish php Pull requests that update Php code technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants