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.

Sign up for GitHub

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

Jump to bottom

[Refactor]: apply same naming convention for all tests that check errors #155

Closed
1 task done
bidzyyys opened this issue Jun 24, 2024 · 7 comments
Closed
1 task done

[Refactor]: apply same naming convention for all tests that check errors #155

bidzyyys opened this issue Jun 24, 2024 · 7 comments
Assignees
@postmeback
Labels
effort: low Easy or tiny task that takes less than a day. good first issue Good for newcomers. priority: 0 Nice-to-have. Willing to ship without this. type: ref A code update that doesn't meaningfully change functionality. type: test Changes to the testing suite.

Comments

@bidzyyys
Copy link
Collaborator

What is the feature you would like to see?

We should apply the same naming convention for all tests that check errors:
fn error_when_<function>_<description>(...)

Contribution Guidelines

  • I agree to follow this project's Contribution Guidelines
@bidzyyys bidzyyys added type: feature New feature request. needs triage Needs to be assigned the appropriate labels labels Jun 24, 2024
@bidzyyys bidzyyys mentioned this issue Jun 24, 2024
Merged
@alexfertel alexfertel added priority: 0 Nice-to-have. Willing to ship without this. type: test Changes to the testing suite. effort: low Easy or tiny task that takes less than a day. type: ref A code update that doesn't meaningfully change functionality. and removed type: feature New feature request. needs triage Needs to be assigned the appropriate labels labels Jun 24, 2024
@alexfertel alexfertel added the good first issue Good for newcomers. label Jun 24, 2024
@postmeback
Copy link

I would like to give it a try.

@alexfertel
Copy link
Contributor

Sure, go for it! @postmeback

@postmeback
Copy link

Can you help with a filename, from where I can get started? @alexfertel

@alexfertel
Copy link
Contributor

Sure, you can start here.

@postmeback postmeback mentioned this issue Jun 27, 2024
Closed
2 tasks
@bidzyyys bidzyyys mentioned this issue Jul 3, 2024
Merged
2 tasks
@alexfertel
Copy link
Contributor

We have decided that the trade-off is not worth it, and it's preferable to surface the intent as clearly as possible in test names. As such, I'll be closing this issue as not planned.

@alexfertel alexfertel closed this as not planned Won't fix, can't repro, duplicate, stale Jul 3, 2024
@github-project-automation github-project-automation bot moved this from Todo to Done in Rust Contracts Stylus Jul 3, 2024
@postmeback
Copy link

Ok 😞

@alexfertel
Copy link
Contributor

Hey @postmeback ! I wrote the reasoning in a comment on your PR. I really hope this doesn't deter you from contributing. It was our mistake and not yours!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@postmeback postmeback

Labels
effort: low Easy or tiny task that takes less than a day. good first issue Good for newcomers. priority: 0 Nice-to-have. Willing to ship without this. type: ref A code update that doesn't meaningfully change functionality. type: test Changes to the testing suite.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

refactor: rename tests with error assertions postmeback/rust-contracts-stylus
3 participants
@postmeback @alexfertel @bidzyyys