Skip to content

Conversation

@darcywong00
Copy link
Contributor

@darcywong00 darcywong00 commented Dec 13, 2023

Addresses this point from keymnapp/keyman.com#403

Refactor environment getenv() calls into module so we have a single global KeymanSiteEnvironment class

TODO: Refactor KeymanHosts?

@darcywong00 darcywong00 added this to the A17S28 milestone Dec 13, 2023
$props = get_class_vars($class);
var_dump($props);
}
}
Copy link
Contributor Author

@darcywong00 darcywong00 Dec 13, 2023

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

  1. unset a property

I'd prefer to rewrite how that particular function works. It's suboptimal at present.

  1. return a boolean if the env var is ''?

No, if the env var is unsset, then the property value should be NULL (testable with is_null, isset, isempty etc).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, if the env var is unsset, then the property value should be NULL ...

I meant for 2. to check when vars are initialized to ''

https://github.com/keymanapp/help.keyman.com/blob/9169d57566ca64026b4421b2fc4a80cc7b7f38b1/_includes/HelpSiteEnvironment.php#L9-L12

Copy link
Member

Choose a reason for hiding this comment

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

If it is needed?

@darcywong00 darcywong00 marked this pull request as draft December 14, 2023 00:51
$props = get_class_vars($class);
foreach($props as $name => $value) {
if (isset($env[$name])) {
self::$instance->$name = $env[$name];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Back to draft since this line is giving me errors

@mcdurdin mcdurdin modified the milestones: A17S28, A17S29 Dec 30, 2023
@mcdurdin mcdurdin modified the milestones: A17S29, A17S30 Jan 6, 2024
@mcdurdin mcdurdin modified the milestones: A17S30, A17S31 Jan 20, 2024
@mcdurdin mcdurdin modified the milestones: A17S31, B17S1 Feb 3, 2024
Base automatically changed from chore/v0.4 to main February 5, 2024 03:26
@mcdurdin mcdurdin modified the milestones: B17S1, B17S2 Feb 17, 2024
@mcdurdin mcdurdin modified the milestones: B17S2, B17S3 Mar 3, 2024
@darcywong00 darcywong00 modified the milestones: B17S3, B17S4 Mar 16, 2024
@mcdurdin mcdurdin modified the milestones: B17S4, B17S5 Mar 30, 2024
@darcywong00 darcywong00 modified the milestones: B17S5, B17S6 Apr 12, 2024
@darcywong00 darcywong00 removed this from the B17S6 milestone Apr 28, 2024
@keyman-server keyman-server modified the milestones: A19S5, A19S6 Jun 22, 2025
@keyman-server keyman-server modified the milestones: A19S6, A19S7 Jul 6, 2025
@keyman-server keyman-server modified the milestones: A19S7, A19S8 Jul 20, 2025
@darcywong00 darcywong00 modified the milestones: A19S8, A19S9 Aug 2, 2025
@darcywong00 darcywong00 modified the milestones: A19S9, A19S10 Aug 16, 2025
@darcywong00 darcywong00 modified the milestones: A19S10, A19S11 Aug 29, 2025
@darcywong00 darcywong00 modified the milestones: A19S11, A19S12 Sep 13, 2025
@darcywong00 darcywong00 modified the milestones: A19S12, A19S13 Sep 27, 2025
@darcywong00 darcywong00 modified the milestones: A19S13, A19S14 Oct 11, 2025
@darcywong00 darcywong00 modified the milestones: A19S14, A19S15 Oct 24, 2025
@keyman-server keyman-server modified the milestones: A19S15, A19S16 Nov 8, 2025
@keyman-server keyman-server modified the milestones: A19S16, A19S17 Nov 22, 2025
@keyman-server keyman-server modified the milestones: A19S17, A19S18 Dec 6, 2025
@keyman-server keyman-server modified the milestones: A19S18, A19S19 Dec 21, 2025
@keyman-server keyman-server modified the milestones: A19S19, A19S20 Jan 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

4 participants