venus icon indicating copy to clipboard operation
venus copied to clipboard

Typo fix: unmarshaling to unmarshalling

Open tuesdayinsighter opened this issue 1 year ago • 7 comments

关联的Issues (Related Issues)

No

改动 (Proposed Changes)

Changes all unmarshaling to unmarshalling

附注 (Additional Info)

自查清单 (Checklist)

在你认为本 PR 满足被审阅的标准之前,需要确保 / Before you mark the PR ready for review, please make sure that:

  • [x] 符合Venus项目管理规范中关于PR的相关标准 / The PR follows the PR standards set out in the Venus project management guidelines
  • [x] 具有清晰明确的commit message / All commits have a clear commit message.
  • [x] 包含相关的的测试用例或者不需要新增测试用例 / This PR has tests for new functionality or change in behaviour or not need to add new tests.
  • [x] 存在兼容性问题(接口, 配置,数据,灰度),如果存在需要进行文档说明 / This PR has compatibility issues (API, Configuration, Data, GrayRelease), if so, need to be documented.
  • [x] 包含相关的的指南以及文档或者不需要新增文档 / This PR has updated usage guidelines and documentation or not need
  • [x] 通过必要的检查项 / All checks are green

tuesdayinsighter avatar May 04 '24 11:05 tuesdayinsighter

Maybe I shouldn't change _gen.go generated code?

https://github.com/filecoin-project/venus/actions/runs/8950231805/job/24613598246

tuesdayinsighter avatar May 06 '24 03:05 tuesdayinsighter

Maybe I shouldn't change _gen.go generated code?

Yes, you should not change the generated code

simlecode avatar May 06 '24 03:05 simlecode

Maybe I shouldn't change _gen.go generated code?

https://github.com/filecoin-project/venus/actions/runs/8950231805/job/24613598246

Just run make gen-all in you local repo to create a new commit for this PR

LinZexiao avatar May 06 '24 03:05 LinZexiao

Codecov Report

Attention: Patch coverage is 0% with 103 lines in your changes are missing coverage. Please review.

Project coverage is 26%. Comparing base (4dd92ba) to head (5c00f04).

:exclamation: Current head 5c00f04 differs from pull request most recent head 77b54cf. Consider uploading reports for the commit 77b54cf to get more accurate results

Additional details and impacted files
@@          Coverage Diff           @@
##           master   #6332   +/-   ##
======================================
- Coverage      26%     26%   -1%     
======================================
  Files         649     649           
  Lines       65519   65519           
======================================
- Hits        17077   17071    -6     
- Misses      45725   45731    +6     
  Partials     2717    2717           

codecov-commenter avatar May 06 '24 03:05 codecov-commenter

Done using make gen-all, it's a little weird that make gen-all changes unmarshalling back to unmarshaling in those generated files.

Steps I've taken:

dnf install hwloc-devel ocl-icd
git submodule update --recursive --checkout

cd extern/filecoin-ffi && make && cd ...
make gen-all

tuesdayinsighter avatar May 06 '24 03:05 tuesdayinsighter

Done using make gen-all, it's a little weird that make gen-all changes unmarshalling back to unmarshaling in those generated files.

It would be better to correct the template for cbor generating to solve this problem. https://github.com/filecoin-project/venus/blob/4dd92baceb19f40862c81f60d102e40dd2ee912e/venus-devtool/cborgen/main.go#L136

LinZexiao avatar May 06 '24 05:05 LinZexiao

Ah, indeed, I've created a PR to cbor-gen at https://github.com/whyrusleeping/cbor-gen/pull/97, we should wait for that PR to merge, update our gen "github.com/whyrusleeping/cbor-gen" in go.mod and consider regenerate those files.

tuesdayinsighter avatar May 06 '24 05:05 tuesdayinsighter

Upstream PR has been closed by author with:

It's the US spelling: https://en.wikipedia.org/wiki/Marshalling_(computer_science)

Maybe we should close this PR too?

tuesdayinsighter avatar May 24 '24 05:05 tuesdayinsighter

The pr https://github.com/whyrusleeping/cbor-gen/pull/97 not accept, so close this.

simlecode avatar May 24 '24 05:05 simlecode