Skip to content

Skill tree additions and restructurings #50

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

Merged
merged 13 commits into from
Aug 10, 2020
Merged

Skill tree additions and restructurings #50

merged 13 commits into from
Aug 10, 2020

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Aug 7, 2020

No description provided.

@RalfJung
Copy link
Member

RalfJung commented Aug 7, 2020

Other things:

  • the "mutable ref in final value" node needs renaming; the feature we want is "mutable ref in const items".
  • <int>::from_str as a specific subitem of FromStr seems rather arbitrary? I'd probably remove it, that seems less confusing.
  • would it make sense to give the feature name everywhere? That makes it easier to match things up. (I have been adding the feature name to issue titles wherever I noticed they are missing, which makes the tracing issues much easier to find in my browser history.)

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 7, 2020

* `<int>::from_str` as a specific subitem of `FromStr` seems rather arbitrary? I'd probably remove it, that seems less confusing.

that has its own issue, thus I added it at all.

* would it make sense to give the feature name everywhere?

yes, but that makes everything more verbose. I've been trimming down the graph where I can

@RalfJung
Copy link
Member

RalfJung commented Aug 7, 2020

that has its own issue, thus I added it at all.

FromStr has a tracking issue. But I was talking about the <int>::from_str.

yes, but that makes everything more verbose. I've been trimming down the graph where I can

I mean it makes it more informative, I wouldn't call that "verbose".^^ When reading a tracking issue, right now it's non-trivial to figure out where in this tree that issue is located.

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 10, 2020

I added all the feature gate names where on existed

@RalfJung
Copy link
Member

RalfJung commented Aug 10, 2020

Awesome!

What does fg: mean? Oh, "feature gate" I guess... isn't it usually called a "feature flag"?
Is just using feature: too long?

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 10, 2020

I moved to feature and did some smaller textual changes that shrink the tree's width again

@RalfJung
Copy link
Member

Thanks a lot! Let's merge this so we can see how it looks rendered.

@RalfJung RalfJung merged commit af332ac into master Aug 10, 2020
@oli-obk oli-obk deleted the skill-tree branch August 10, 2020 10:48
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