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

Jump to bottom

Update system.xml #276

Merged
merged 3 commits into from
Dec 13, 2023
Merged

Update system.xml #276

merged 3 commits into from
Dec 13, 2023

Conversation

DomPixie
Copy link
Contributor

Don't put Klaviyo above important core items

Description

It's being nice to other developers and the Magento team

Pre-Submission Checklist:

  • You've updated the CHANGELOG following the steps here
  • Internal Only - If this is a release, please confirm the following:
    • The links in the changelog have been updated to point towards the new versions
    • The version has been incremented in the following places: module.xml and composer.json

NOTE: Please use the Changelogger cli tool to manage versioned file upgrades.

@DomPixie
Update system.xml
1752ffc 
Don't put Klaviyo above important core items
@DomPixie DomPixie requested a review from a team as a code owner November 30, 2023 15:15
@klaviyoit klaviyoit requested a review from chavesj November 30, 2023 15:15
@DomPixie
Copy link
Contributor Author

Should Klaviyo become the default when you click Stores > Settings > Configuration?

I think not

@chavesj
Copy link
Contributor

chavesj commented Dec 5, 2023

@DomPixie How was this tested? Please provide more information re: your validation of these changes, including but not limited to a screenshot of the settings / config page

@DomPixie
Copy link
Contributor Author

DomPixie commented Dec 6, 2023

@chavesj

This stops klaviyo becoming default under Stores > Settings > Configuration

image

Default
image

chavesj
chavesj suggested changes Dec 6, 2023
View reviewed changes
Copy link
Contributor

@chavesj chavesj left a comment

Choose a reason for hiding this comment

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

Please update the CHANGELOG

@DomPixie
Copy link
Contributor Author

DomPixie commented Dec 7, 2023

@chavesj
Updated. Although I will take your lead on this kind of thing. Thanks.

@DomPixie DomPixie requested a review from chavesj December 11, 2023 11:11
chavesj
chavesj approved these changes Dec 13, 2023
View reviewed changes
Copy link
Contributor

@chavesj chavesj left a comment •
edited
Loading

Choose a reason for hiding this comment

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

:shipit:

@smoucka smoucka mentioned this pull request Dec 13, 2023
Merged
4 tasks
@chavesj chavesj merged commit 3cc7ea9 into klaviyo:master Dec 13, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chavesj chavesj chavesj approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@DomPixie @chavesj