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

Reuse fields #30

Closed
wants to merge 2 commits into from
Closed

Reuse fields #30

wants to merge 2 commits into from

Conversation

BenBroide
Copy link

#23

@atanas-dev
Copy link

Thank you for the contribution @BenBroide , however there are a few items that have to be addressed:

  1. Pull requests should do one thing and one thing only - this PR introduced 2 unrelated changes
  2. The Important note change is already covered in Theme Options and User Meta container documentation (second sentence on https://carbonfields.net/docs/containers-theme-options/?crb_version=2-1-0)

Since I have to reject one of the changes now I have to reject both since they are in the same PR.
If you wish to contribute the Reuse fields page separately, please follow these steps:

  1. Expand on the description and benefits of doing this
  2. Reduce the length of the example code
  3. Add a link to the new article in documentation/40-advanced-topics/README.md
  4. Create a fresh PR with only the Reuse fields page

Thanks again!

@atanas-dev atanas-dev closed this Jan 4, 2018
@BenBroide
Copy link
Author

@atanas-angelov-dev was in my code from a different PR 29, #29 which didn't receive feedback yet.
It would be very beneficial to add the note from that branch in "conditional disaply" page becuse that where users would look for that information.
The reason I sent that PR 29 is because a few times colleagues asked me the same question. Beyond that, 3 issues were open about this that I linked in PR 29.
I think It would be really useful to add that note, not all users know to look about a conditional display issue in Theme Options docs page or User Meta docs page.
Thanks for this great plugin.

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