Skip to content

Add PIET to the list of Solidity tools#7541

Closed
hellraiserinchief wants to merge 1 commit intoargotorg:developfrom
hellraiserinchief:develop
Closed

Add PIET to the list of Solidity tools#7541
hellraiserinchief wants to merge 1 commit intoargotorg:developfrom
hellraiserinchief:develop

Conversation

@hellraiserinchief
Copy link
Copy Markdown
Contributor

@hellraiserinchief hellraiserinchief commented Oct 16, 2019 •
edited
Loading

Description

Checklist

  • Code compiles correctly
  • All tests are passing
  • New tests have been created which fail without the change (if possible)
  • README / documentation was extended, if necessary
  • Changelog entry (if change is visible to the user)
  • Used meaningful commit messages

@hellraiserinchief hellraiserinchief mentioned this pull request Oct 16, 2019
Closed
chriseth
chriseth previously approved these changes Oct 16, 2019
View reviewed changes
chriseth
chriseth reviewed Oct 16, 2019
View reviewed changes
docs/resources.rst Outdated

* `Universal Mutator <https://github.com/agroce/universalmutator>`_
A tool for mutation generation, with configurable rules and support for Solidity and Vyper.

Copy link
Copy Markdown
Contributor

@chriseth chriseth Oct 16, 2019

Choose a reason for hiding this comment

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

Our tests are complaining about trailing whitespace here - can you please check?

@chriseth chriseth dismissed their stale review October 16, 2019 08:07

Test failures

@hellraiserinchief
Copy link
Copy Markdown
Contributor Author

hellraiserinchief commented Oct 16, 2019

All checks seem to pass now

chriseth
chriseth previously approved these changes Oct 17, 2019
View reviewed changes
@chriseth
Copy link
Copy Markdown
Contributor

chriseth commented Oct 17, 2019

Could you please squash it into a single commit?

erak
erak previously approved these changes Oct 18, 2019
View reviewed changes
@erak erak mentioned this pull request Oct 18, 2019
Merged
@erak
Copy link
Copy Markdown
Collaborator

erak commented Oct 18, 2019

@hellraiserinchief Could you please rebase? We just fixed the broken MacOS build on Circle which led to the failing test on this PR.

@erak erak mentioned this pull request Oct 18, 2019
Merged
6 tasks
erak
erak requested changes Oct 18, 2019
View reviewed changes
Copy link
Copy Markdown
Collaborator

@erak erak left a comment

Choose a reason for hiding this comment

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

I'm sorry, but the commits are bit messed up and do appear twice. Did you rebase onto develop? Do you need help with this?

@hellraiserinchief
Copy link
Copy Markdown
Contributor Author

hellraiserinchief commented Oct 19, 2019

I'm sorry, but the commits are bit messed up and do appear twice. Did you rebase onto develop? Do you need help with this?

Yeah, help would be welcome, I am new to contributing to OSS

@hellraiserinchief
Copy link
Copy Markdown
Contributor Author

hellraiserinchief commented Oct 19, 2019

I'm sorry, but the commits are bit messed up and do appear twice. Did you rebase onto develop? Do you need help with this?

Yeah, help would be welcome, I am new to contributing to OSS

I have tried combined the commits into a single commit, please have a look and advice.

@Marenz Marenz requested a review from erak October 21, 2019 15:01
@chriseth
Copy link
Copy Markdown
Contributor

chriseth commented Oct 22, 2019 •
edited
Loading

Sorry, I fear you have to rebase again, the macos version change should be part of a different PR.

@erak
Copy link
Copy Markdown
Collaborator

erak commented Oct 23, 2019

As @chriseth already mentioned, this needs to be rebased again, such that the changes to .circleci/config.yaml are not part of the PR. Also, since your working on develop of your fork, I'd strongly recommend to create a branch in your forked repository, and then open a PR with our develop as the target and your branch as the source. Your current workflow might lead to issues when trying to keep your fork up to date. Please see https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/syncing-a-fork for further advice.

erak
erak requested changes Oct 28, 2019
View reviewed changes
Copy link
Copy Markdown
Collaborator

@erak erak left a comment

Choose a reason for hiding this comment

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

@hellraiserinchief Could you also remove the change to .circleci/config.yml? Otherwise this PR looks good!

@hellraiserinchief
Copy link
Copy Markdown
Contributor Author

hellraiserinchief commented Oct 30, 2019

@erak, I will follow your advice and create a new branch to submit a new PR, instead of using the develop branch directly

@hellraiserinchief hellraiserinchief mentioned this pull request Oct 30, 2019
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@erak erak erak requested changes

@Marenz Marenz Awaiting requested review from Marenz

+1 more reviewer

@chriseth chriseth chriseth left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@hellraiserinchief @chriseth @erak @Marenz