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.

Sign up for GitHub

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

refactor: Add KeymanSiteEnvironment #12

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
+83 −0
Draft

refactor: Add KeymanSiteEnvironment #12

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f834646
refactor: Add site environment
darcywong00 Dec 12, 2023
e0a4910
Add KeymanSiteEnvironment to _bootstrap_configure_common
darcywong00 Dec 12, 2023
3f1fcce
chore: Cleanup unused code
darcywong00 Dec 12, 2023
360ffe4
fix: Add IsSet
darcywong00 Dec 13, 2023
b3bbaa2
fix: Check for null
darcywong00 Dec 13, 2023
103900c
fix: Use dynamic properties
darcywong00 Dec 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions _common/KeymanSiteEnvironment.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php
declare(strict_types=1);

namespace Keyman\Site\Common;

class KeymanSiteEnvironment {

protected static function _Init($class) {
$env = getenv();
$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

}
}
}

public static function Debug($class) {
$props = get_class_vars($class);
var_dump($props);
}

public static function IsSet($name) {
if (isset(self::$instance->$name) && !is_null(self::$instance->$name)) {
return true;
}
return false;
}
}
Copy link
Contributor Author

@darcywong00 darcywong00 Dec 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Looking ahead to refactoring

https://github.com/keymanapp/api.keyman.com/blob/9bb62cc86edd9b25fc765bf78c18120e7aed1002/tests/TestDBBuild.inc.php#L49-L51

and

https://github.com/keymanapp/api.keyman.com/blob/9bb62cc86edd9b25fc765bf78c18120e7aed1002/tests/TestUtils.inc.php#L18-L22

Will we want additional functions to

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

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?

1 change: 1 addition & 0 deletions bootstrap.inc.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ function _bootstrap_configure_common() {
JsonApiFailure.php
KeymanHosts.php
KeymanSentry.php
KeymanSiteEnvironment.php
MarkdownHost.php
)
local common_file=
Expand Down