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.

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

Change assert() to FC_ASSERT. #669

Closed
wants to merge 1 commit into from

Conversation

marcialvieira
Copy link
Contributor

Another commit to the issue #511

@abitmore
Copy link
Member

@marcialvieira thanks! Please follow the steps described in #511 (just updated) to see if the changes will work.

@abitmore
Copy link
Member

Need to change the base branch to hardfork after #670 get merged.

@abitmore abitmore changed the base branch from develop to hardfork February 15, 2018 09:07
@abitmore abitmore added the 2a Discussion Needed Prompt for team to discuss at next stand up. label Apr 17, 2018
@pmconrad
Copy link
Contributor

pmconrad commented May 8, 2018

IMO it is too dangerous to simply search&replace. Each of these asserts needs to be carefully reviewed

  • if it is (still) correct
  • if it is a useful check

We should close this PR and rather remove/replace assert's whenever we come across them. Once we've got rid of most of them we can tackle the rest in one go.

@abitmore
Copy link
Member

abitmore commented May 8, 2018

@pmconrad agreed. Closing.

@marcialvieira thanks for contribution anyway. Sorry for wasted your time/efforts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2a Discussion Needed Prompt for team to discuss at next stand up.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants