Introduce taxonomy equivalent of menu map #1187
Conversation
✅ Deploy Preview for cyf-common canceled.
|
✅ Deploy Preview for cyf-curriculum canceled.
|
✅ Deploy Preview for cyf-programming ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for cyf-sdc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for cyf-launch ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for cyf-tracks ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for cyf-piscine ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for cyf-itd canceled.
|
illicitonion
force-pushed
the
introduce-taxonomy-map
branch
3 times, most recently
from
009df09 to
2b4f00a
Compare
This produces the same view (sharing code to do so), but allows using a taxonomy instead of a list of menus. The benefit here is a taxonomy gives you a place (the taxonomy term page) to define metadata that may be useful in presenting each term (e.g. attaching a description to each term). The equivalences between menu and taxonomy maps are: | Concept | Menu concept | Taxonomy concept | |---------------------------------------------------------------|------------------|-----------------------| | A category (e.g. a column to display in the default map view) | A menu | A term | | A value in the category (e.g. a a card to show in the column) | A page in a menu | A value within a term | There are some interesting compatibility questions in here which I will comment on in the PR I'm raising for this. Note: This currently doesn't actually define any interesting metadata (other than weight) for each term in the tracks taxonomy - I will send this through as a follow-up PR, to separate out "the idea of a taxonomy map" from "the details of this particular taxonomy".
This is just to show that it still works. I will re-apply the update before merging.
illicitonion
force-pushed
the
introduce-taxonomy-map
branch
from
2b4f00a to
0a92a98
Compare
|
@illicitonion I don't see any comments in the PR other than the inline comments in the code, and I didn't see any specific questions in those. In general, this all seems reasonable to me. Seems like a reasonable change, and including some backwards compatibility with a warning also seems reasonable to me. Is there something more specific I should be giving feedback on? |
A reasonable question! I think my specific questions are:
|
|
Those are indeed more specific. :) So first, let me say that from a community perspective, I'm of the opinion that it's much more important to decide on a release cadence and communicate it well than whatever the actual cadence is. I haven't been around CYF long enough to know what you guys would consider a reasonable schedule given your staff and resource constraints, so I don't think that's a determination I can make. Also, with my former PM hat on, I feel like this is straying into product decision territory. I think it would be best to loop in @kfklein15 on this discussion. My particular (by no means definitive) opinions on your questions:
I hope it's a bit helpful to get a relative outsider's opinion, but again, I don't think I'm the right person to be dictating these decisions. I'm happy to document and support whatever direction CYF would like to go with a release cadence. I don't think the answers to these questions should hold up merging a PR, personally, but I'll wait for you to confirm that this is a satisfactory "review" before I approve it. I don't want to automerge anything you guys aren't ready for. |
|
I think that all makes sense, thanks @daslerr! I'll leave for @SallyMcGrath to review and we can merge this - thanks for talking through this as a case study with me! |
|
Great convo here both of you, thanks. Breaking change probably means you can't build your site when the only change you have made is updating the version from us. That's the same things said from the other side. In a call with Reshama more may be forthcoming. |
reshamas
added
the
Breaking Change
|
@illicitonion |
|
|
Thanks for the discussion all!
Interesting... I put in some work to make this not currently a breaking change - is the idea that we're collecting "things which introduce deprecations" as breaking changes? Or should I remove those back-compat affordances and make this a breaking change? Or should I remove the label, and add it in two cycles when we remove the deprecated back-compat code?
Works for me! Just to be clear, does that mean: Version n: Introduce deprecation ?
So in this example, if I remove the special-casing for looking for the And if we do, are we effectively saying our policy is: You should upgrade versions one at a time and address deprecation warnings as they come up. If you jump versions, things may break in ways you're not expecting without warning. |
illicitonion
removed
the
Breaking Change
There was a problem hiding this comment.
Oh ugh! Apparently GitHub left my line comments as pending - I'm sorry @daslerr, I understand why you were confused about them now! Here they are for posterity!
This reverts commit 0a92a98.
Ah, yeah, that makes a lot more sense now. :) Thanks! |
What does this change?
Introduce taxonomy equivalent of menu map
This produces the same view (sharing code to do so), but allows using a taxonomy instead of a list of menus. The benefit here is a taxonomy gives you a place (the taxonomy term page) to define metadata that may be useful in presenting each term (e.g. attaching a description to each term).
The equivalences between menu and taxonomy maps are:
There are some interesting compatibility questions in here which I will comment on in the PR I'm raising for this.
Note: This currently doesn't actually define any interesting metadata (other than weight) for each term in the tracks taxonomy - I will send this through as a follow-up PR, to separate out "the idea of a taxonomy map" from "the details of this particular taxonomy".
Common Theme?
See description above, and line comments for specific considerations.
Org Content?
Updates org-cyf-tracks to use a taxonomy not a menu-map.
Checklist
Who needs to know about this?
cc @CodeYourFuture/dpg-team - I've left comments with some compatibility questions that seem relevant to the releases discussions.