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

added variables so checkboxes on theme for sbh2 are updated correctly… #1598

Closed
wants to merge 7 commits into from

Conversation

danielfang97
Copy link
Collaborator

… according to config

@danielfang97 danielfang97 requested a review from cjmyers December 16, 2024 02:47
@cjmyers
Copy link
Collaborator

cjmyers commented Dec 16, 2024

Seems to be failing testing. Also, not sure about the logic. Can you explain it?

@danielfang97
Copy link
Collaborator Author

Re ran tests, and now it seems to be passing tests.
I did this because for SBH2, it wasn't passing in the variables to SBH1. So lines 50-52 are adding the variables to theme, so they can be accessed by SBH2.
For the other changes, I am setting the variable based on whether it's true or false. The issue is that I realized that SBH1 does this differently. I think this would be good to discuss tomorrow.

@cjmyers
Copy link
Collaborator

cjmyers commented Dec 17, 2024

Rethink and retest the logic for setting the variables. Remove tests that are not necessary for getting this to pass.

@danielfang97
Copy link
Collaborator Author

Closed, as this PR was replaced with PR #1601

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.

2 participants