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

Planet 2561 #738

Closed
wants to merge 40 commits into from
Closed

Planet 2561 #738

wants to merge 40 commits into from

Conversation

onyekaa
Copy link

@onyekaa onyekaa commented Mar 22, 2019

Master branch of all changes made for Planet 2561. Merged in the deploy branch.
Changes include:

  • Exporter functionality
  • The metaboxes for campaign theming
  • The Campaign post type twig and php files.

mehulkaklotar and others added 30 commits January 28, 2019 17:56
- Placed the list of themes in one function for use in the Post Campaign Class
- More formatting fixes
- Removed unneeded blocks from single-campaigns.
- Reset changes to gulpfile
* Added CPT for campaigns and page templates

* Syntax fix and removal of unnecessary scss

* Added CPT for campaigns and page templates

* Formatting and split Campaign template class into new file

* Removed unneeded code and resetting changes

- Placed the list of themes in one function for use in the Post Campaign Class
- More formatting fixes
- Removed unneeded blocks from single-campaigns.
- Reset changes to gulpfile

* Linting and formatting fixes

* Remove fonts, icon and reset style map.
- Page template support added
- Font/logo support added

Delete obsolete style files.

Remove unneeded images
@onyekaa onyekaa added the WIP label Mar 22, 2019
# Conflicts:
#	templates/html-header.twig
@comzeradd
Copy link
Member

This has similar issues with the plugin-blocks PR. It's full with fixup commits and merge commits from the target branch.

I see this is still WIP. But when it's ready to be merged, we should spend some time to clean up the commits.

@onyekaa
Copy link
Author

onyekaa commented Mar 26, 2019

@comzeradd I'm basically done with it. You want me to squash just the merge commits? Or which other ones?

@onyekaa onyekaa added Review and removed WIP labels Mar 26, 2019
@comzeradd
Copy link
Member

The merge commits that were created as part of merging PRs to this branch are fine. The ones that shouldn't be there are ones that merge the target branch back into this. (eg. Merge branch 'develop' into planet-2561).

Additionally the PR has many fixup commits (eg. Linter fixes, Formatting fixes, etc). In general we avoid having commits that fix syntax, linting, typo errors that were introduced on previous commits of the same PR.

@kirdia
Copy link
Contributor

kirdia commented Mar 29, 2019

Ideally these commits (to which Nikos is referring to) should not exist as they pollute the git history
For example these commits
commit 2d6d2ea
commit 0c72f63
commit e61ffa5 (pull/703)

As this is an evolving codebase, we need to keep the git history as clean as possible.
And while these commits have some value during the lifeline of the branch/feature development they don't have any value for us after the feature is completed and merged and makes it harder to maintain the code.

@comzeradd
Copy link
Member

Closing this in favor of #751

@comzeradd comzeradd closed this Apr 4, 2019
@comzeradd comzeradd deleted the planet-2561 branch April 8, 2019 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants