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

chore: Add info.php to show Tier 🦭 #211

Merged
merged 2 commits into from
Jan 24, 2024
Merged

chore: Add info.php to show Tier 🦭 #211

merged 2 commits into from
Jan 24, 2024

Conversation

darcywong00
Copy link
Contributor

Relates to keymanapp/keyman.com#403

verify TIER on k8s (_control/info.php as used on other sites)

Note, the autoload.php here doesn't register a namespace ^Keyman\\\\Site\\\\com\\\\keyman\\\\s\\\\(.+)...

@darcywong00 darcywong00 added this to the A17S30 milestone Jan 19, 2024
Copy link
Member

@mcdurdin mcdurdin left a comment

Choose a reason for hiding this comment

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

Almost LGTM.

I am not keen on adding more php code to s.keyman.com, because this site is intended to be completely static. This leads us down a difficult path to dig out of if we want to put this on a formal CDN in the future... This is particularly true if we start depending on the _common files, which shouldn't be on s.keyman.com at all if possible.

Is it possible to just have _control/info.php which reads the tier directly? I know it means code duplication but it avoids introducing dependencies that will make our life hard.

Remember that we are removing the other .php deployment-related code in staging in the future, and hopefully will remove the kmwversion.php one day as well.

@darcywong00
Copy link
Contributor Author

This is particularly true if we start depending on the _common files, which shouldn't be on s.keyman.com at all if possible.

I think currently boostrap_configure is pulling the entire _common/ folder which includes .sh and .php files. Are you saying you'd like to avoid using _common/*.php files (e.g. _common/KeymanHosts.php)?

Is it possible to just have _control/info.php which reads the tier directly?

I think I could copy these lines into info.php
https://github.com/keymanapp/shared-sites/blob/bcc0f472439a0b3d03541aaf46004351f495d35d/_common/KeymanHosts.php#L83-L93

@mcdurdin
Copy link
Member

I think currently boostrap_configure is pulling the entire _common/ folder which includes .sh and .php files. Are you saying you'd like to avoid using _common/*.php files (e.g. _common/KeymanHosts.php)?

Yes, it's okay if those files are being pulled in for now. I'd just like to avoid building a dependency on them. (So those files are effectively ignored and irrelevant; in fact we could have a bootstrap mode that doesn't pull in _common)

I think I could copy these lines into info.php
https://github.com/keymanapp/shared-sites/blob/bcc0f472439a0b3d03541aaf46004351f495d35d/_common/KeymanHosts.php#L83-L93

Yes, sounds good.

@mcdurdin mcdurdin modified the milestones: A17S30, A17S31 Jan 20, 2024
@darcywong00 darcywong00 merged commit 4fe0f8a into staging Jan 24, 2024
3 checks passed
@darcywong00 darcywong00 deleted the chore/tier-info branch January 24, 2024 23:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants