sway icon indicating copy to clipboard operation
sway copied to clipboard
verified publisher icon FuelLabs

fix some comments

Open cuishuang opened this issue 1 year ago • 9 comments

Description

Checklist

  • [x] I have linked to any relevant issues.
  • [x] I have commented my code, particularly in hard-to-understand areas.
  • [x] I have updated the documentation where relevant (API docs, the reference, and the Sway book).
  • [x] I have added tests that prove my fix is effective or that my feature works.
  • [x] I have added (or requested a maintainer to add) the necessary Breaking* or New Feature labels where relevant.
  • [x] I have done my best to ensure that my PR adheres to the Fuel Labs Code Review Standards.
  • [x] I have requested a review from the relevant team or maintainers.

cuishuang avatar Mar 03 '24 02:03 cuishuang

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Mar 03 '24 02:03 CLAassistant

@cuishuang I'm not sure why those tests failed. Please update your branch as it is now out of date.

sdankel avatar Apr 19 '24 05:04 sdankel

@cuishuang I'm not sure why those tests failed. Please update your branch as it is now out of date.

Thanks. Updated.

cuishuang avatar Apr 28 '24 02:04 cuishuang

Hi, thanks for the PR. CI seems to be failing because you updated an error message but did not changed the test for that error message. So what you need to do make the pipeline green is changing the expected message at the related test.toml.

Modified. Thanks!

cuishuang avatar Apr 29 '24 02:04 cuishuang

Anything need I to do? Please feel free to tell me.

cuishuang avatar May 01 '24 10:05 cuishuang

Solve the merge conflicts and we should be able to merge this.

IGI-111 avatar May 01 '24 13:05 IGI-111

Solve the merge conflicts and we should be able to merge this.

Solved, please review again.

cuishuang avatar May 02 '24 06:05 cuishuang

It's outdated again :sweat:

IGI-111 avatar May 06 '24 16:05 IGI-111

It's outdated again 😓

Updated! Please review again.

cuishuang avatar May 07 '24 09:05 cuishuang

The HEAD is moving pretty fast so it's outdated again, please check the box that allows edits by maintainers in this PR so that we can auto update it and it can be merged.

IGI-111 avatar May 08 '24 17:05 IGI-111

The HEAD is moving pretty fast so it's outdated again, please check the box that allows edits by maintainers in this PR so that we can auto update it and it can be merged.

Conflicts have been resolved and code updated.

Since I forked the project to an organization rather than to my personal account, I couldn't see the "allows edits by maintainers" option. I tried a lot and finally checked the documentation and discussions and found that from fork to organization, the project does not have this feature. Even though I created this organization and am the only member.

https://github.com/orgs/community/discussions/5634

Sorry for causing this trouble. If the problem still cannot be solved, I can close this PR, fork it to my personal account, and submit a new PR.

Waiting for your reply and decision

image

cuishuang avatar May 09 '24 02:05 cuishuang

I'm going to close this PR, please reopen one with a personal account fork and we'll get this merged.

IGI-111 avatar May 09 '24 20:05 IGI-111

I'm going to close this PR, please reopen one with a personal account fork and we'll get this merged.

Thanks. The new pr https://github.com/FuelLabs/sway/pull/5989

The new pr allow edits by maintainers, Please review again.

cuishuang avatar May 11 '24 02:05 cuishuang