Skip to content

Conversation

burrbull
Copy link
Member

No description provided.

@burrbull burrbull marked this pull request as ready for review May 11, 2022 11:34
@burrbull burrbull requested a review from a team as a code owner May 11, 2022 11:34
@burrbull
Copy link
Member Author

cc @Emilgardis

};

#[derive(Clone, Debug, PartialEq, Hash, Eq)]
pub struct RPath {
Copy link
Member

Choose a reason for hiding this comment

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

I think it's ok to have these have a longer name

Suggested change
pub struct RPath {
pub struct RegisterPath {

keeping it is fine too though, it may just be unclear for someone not familiar with how this works and looking here first time

This comment was marked as outdated.

}

#[derive(Clone, Debug, PartialEq, Hash, Eq)]
pub struct FPath {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pub struct FPath {
pub struct FieldPath {

#[derive(Clone, Debug, PartialEq, Hash, Eq)]
pub struct FPath {
register: RPath,
field: String,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
field: String,
ident: String,

or

Suggested change
field: String,
name: String,

@burrbull burrbull marked this pull request as draft May 11, 2022 17:00
@burrbull burrbull marked this pull request as ready for review May 11, 2022 19:19
@burrbull
Copy link
Member Author

Now it should look much cleaner

@burrbull
Copy link
Member Author

cc @Emilgardis

Copy link
Member

@Emilgardis Emilgardis left a comment

Choose a reason for hiding this comment

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

mb! thought I r+ ed this earlier!

bors r+

@bors
Copy link
Contributor

bors bot commented May 12, 2022

Build succeeded:

@bors bors bot merged commit 60d435f into master May 12, 2022
@bors bors bot deleted the nested branch May 12, 2022 21:13
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