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

derive setters for spec and mount #76

Merged
merged 2 commits into from
Oct 5, 2021
Merged

Conversation

yihuaf
Copy link
Contributor

@yihuaf yihuaf commented Oct 4, 2021

As discussed in #74, this will greatly reduce the need to copy spec/mount when modifying these structures. Leaving the rest as is at the moment. Hopefully, this is enough.

@yihuaf
Copy link
Contributor Author

yihuaf commented Oct 4, 2021

@Furisto @utam0k PTAL

@codecov-commenter
Copy link

Codecov Report

Merging #76 (aa4d13c) into main (9c4327b) will not change coverage.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##             main      #76   +/-   ##
=======================================
  Coverage   26.93%   26.93%           
=======================================
  Files          19       19           
  Lines        1344     1344           
  Branches      694      694           
=======================================
  Hits          362      362           
  Misses        374      374           
  Partials      608      608           

Copy link
Member

@utam0k utam0k left a comment

Choose a reason for hiding this comment

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

LGTM

@utam0k
Copy link
Member

utam0k commented Oct 5, 2021

I'll wait for the merge until we get @saschagrunert's review.

Copy link
Contributor

@saschagrunert saschagrunert left a comment

Choose a reason for hiding this comment

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

LGTM, following up and deriving it for all before releasing a new version seems to make sense to me for consistency reasons.

@utam0k utam0k mentioned this pull request Oct 5, 2021
@utam0k
Copy link
Member

utam0k commented Oct 5, 2021

LGTM, following up and deriving it for all before releasing a new version seems to make sense to me for consistency reasons.

@saschagrunert I agree with you. I created the issue(#77) to respond to this.

@utam0k utam0k merged commit 19c26ca into youki-dev:main Oct 5, 2021
@yihuaf yihuaf deleted the yihuaf/setters branch October 5, 2021 15:36
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.

4 participants