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

refactor(includes): logging-functions exclude wp_login action from logging #1

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

paulkirspuu
Copy link
Collaborator

GH repo version vs live is different, my proposed version is what would go live.

We should probably first update repo, recommit and go from there. For now I commited this so we can move fast with what's important.

…gging

GH repo version vs live is different, my proposed version is what would go live.

We should probably first update repo, recommit and go from there. For now commited so we can move fast with what's important.
@pawelkmpt
Copy link

As far as I see GH version is 3.7.0 and live site 1.4.11 so huge mismatch. I think we either have to create repo or branch for 1.4 . Otherwise this PR doesn't make sense.

@lkraav
Copy link

lkraav commented Jan 2, 2024

Yes, all work must be based on 1.4 branch, which our fork already holds.

But indeed, warmpress copy should be re-checked vs live copy. Can't recall any reason for divergence off the top.

Note: GH repo version vs live is different, my proposed version is what matches live.
@paulkirspuu
Copy link
Collaborator Author

Quite straightforward this one 4c9627b.

Don't see how it could be done without interfering plugin side, and since its deprecated anyway, no point in complicating things.

@paulkirspuu
Copy link
Collaborator Author

paulkirspuu commented Jan 12, 2024

I can deploy these 2 commits live now, I'd estimate they're harmless enough to do this on a Friday.

Could run DB cleanup in weekend then, or part of it.

@pawelkmpt
Copy link

pawelkmpt commented Jan 15, 2024

Yes, all work must be based on 1.4 branch, which our fork already holds.

@lkraav already holds where? What did you mean? Repo tag?

I can deploy these 2 commits live now, I'd estimate they're harmless enough to do this on a Friday.

@paulkirspuu did you deploy?

Could you post diff screenshot between updates and live site?

@paulkirspuu
Copy link
Collaborator Author

Deployed now, per https://cxlworld.slack.com/archives/C01HXU5GGJG/p1705316582199729?thread_ts=1703154651.995509&cid=C01HXU5GGJG.

Could you post diff screenshot between updates and live site?

image

This is vs live.

Change in overall code between live vs repo is minimal and irrelevant for small changes like these, but if we were to do something bigger in patching BadgeOS, we'd need to sync the two (99% certain we won't).

@pawelkmpt pawelkmpt changed the base branch from master to cxl-1.4 January 15, 2024 12:31
@pawelkmpt pawelkmpt changed the base branch from cxl-1.4 to master January 15, 2024 12:32
@pawelkmpt
Copy link

@paulkirspuu @lkraav I created branch cxl-1.4 based on https://github.com/opencredit/badgeos/releases/tag/1.4.11 and using 5877d41 hash.

Changing base of this PR doesn't help as it carries over hundreds of commits (as it was branched off from 3.7.x) so branch would need to be re-done.

Before it'd be good if @paulkirspuu compared cxl-1.4 with the live site. If any differences present - commit them, then file updated PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants