Skip to content

Add PIET to the list of Solidity tools - #7541

Closed
hellraiserinchief wants to merge 1 commit into
argotorg:developfrom
hellraiserinchief:develop
Closed

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

Conversation

@hellraiserinchief

@hellraiserinchief hellraiserinchief commented Oct 16, 2019

Copy link
Copy Markdown
Contributor

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

chriseth
chriseth previously approved these changes Oct 16, 2019
Comment thread 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

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

All checks seem to pass now

chriseth
chriseth previously approved these changes Oct 17, 2019
@chriseth

Copy link
Copy Markdown
Contributor

Could you please squash it into a single commit?

erak
erak previously approved these changes Oct 18, 2019
@erak

erak commented Oct 18, 2019

Copy link
Copy Markdown
Collaborator

@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 left a comment

Copy link
Copy Markdown
Collaborator

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

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

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

chriseth commented Oct 22, 2019

Copy link
Copy Markdown
Contributor

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

@erak

erak commented Oct 23, 2019

Copy link
Copy Markdown
Collaborator

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 left a comment

Copy link
Copy Markdown
Collaborator

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

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

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