Skip to content

CI configuration refactor#4776

Merged
alessio merged 4 commits intomasterfrom
mircea/update-ci-config
Jul 25, 2019
Merged

CI configuration refactor#4776
alessio merged 4 commits intomasterfrom
mircea/update-ci-config

Conversation

@mircea-c
Copy link
Copy Markdown

@mircea-c mircea-c commented Jul 24, 2019 •
edited
Loading

Closes #4775

  • Linked to github-issue with discussion and accepted design OR link to spec that describes this work.
  • Wrote tests
  • Updated relevant documentation (docs/)
  • Added a relevant changelog entry: clog add [section] [-t <tag>] [-m <msg>]
  • Re-reviewed Files changed in the github PR explorer

For Admin Use:

  • Added appropriate labels to PR (ex. wip, ready-for-review, docs)
  • Reviewers Assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@mircea-c mircea-c changed the title R4R: CI configuration refactor R4R: Jul 24, 2019
@mircea-c mircea-c changed the title R4R: CI configuration refactor Jul 24, 2019
@codecov
Copy link
Copy Markdown

codecov bot commented Jul 24, 2019 •
edited
Loading

Codecov Report

Merging #4776 into master will decrease coverage by 3.13%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4776      +/-   ##
==========================================
- Coverage   53.89%   50.75%   -3.14%     
==========================================
  Files         270      291      +21     
  Lines       17230    18687    +1457     
==========================================
+ Hits         9286     9485     +199     
- Misses       7260     8511    +1251     
- Partials      684      691       +7

jackzampolin
jackzampolin approved these changes Jul 24, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@jackzampolin jackzampolin left a comment

Choose a reason for hiding this comment

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

Nice cleanup of our circle stuff @mircea-c !!

alexanderbez
alexanderbez reviewed Jul 24, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@alexanderbez alexanderbez left a comment •
edited
Loading

Choose a reason for hiding this comment

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

What is the idea behind the changes to tooling and Makefile?

@mircea-c
Copy link
Copy Markdown
Author

mircea-c commented Jul 24, 2019

@alexanderbez there's only 2 other tools that are in use right now. The runsim tool which handles running long simulations, and clog. I don't think either of those should be pulled in as dependencies for the SDK.

golangci-lint is being run through github. I don't think having a specific target for that in the makefile brings much, if any, value.

fedekunze
fedekunze approved these changes Jul 25, 2019
View reviewed changes
@mircea-c mircea-c force-pushed the mircea/update-ci-config branch 4 times, most recently from 2c88657 to 32a758c Compare July 25, 2019 17:07
@mircea-c
Copy link
Copy Markdown
Author

mircea-c commented Jul 25, 2019

@alexanderbez I've added back and fixed the tools target. Everything works as we discussed yesterday.

@mircea-c mircea-c force-pushed the mircea/update-ci-config branch from 32a758c to cf49b5e Compare July 25, 2019 17:18
alessio
alessio suggested changes Jul 25, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@alessio alessio left a comment

Choose a reason for hiding this comment

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

Just one tiny typo - looks good otherwise

@mircea-c mircea-c force-pushed the mircea/update-ci-config branch 4 times, most recently from ec339d2 to 96dd331 Compare July 25, 2019 17:44
@alessio alessio merged commit e8ed930 into master Jul 25, 2019
@alessio alessio deleted the mircea/update-ci-config branch July 25, 2019 18:21
alexanderbez pushed a commit that referenced this pull request Aug 2, 2019
@alexanderbez
CI configuration refactor (#4776)
318e7cc 
* added back the tools targets
* removed ci target
fedekunze
fedekunze reviewed Aug 6, 2019
View reviewed changes
Makefile
@go test -mod=readonly $(SIMAPP) -run TestFullAppSimulation -Genesis=${HOME}/.gaiad/config/genesis.json \
-Enabled=true -NumBlocks=100 -BlockSize=200 -Commit=true -Seed=99 -Period=5 -v -timeout 24h

test_sim_app_fast:
Copy link
Copy Markdown
Contributor

@fedekunze fedekunze Aug 6, 2019

Choose a reason for hiding this comment

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

@mircea-c why was this deleted?

Copy link
Copy Markdown
Contributor

@fedekunze fedekunze Aug 6, 2019

Choose a reason for hiding this comment

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

either this was accidental or it wasn't removed from the PHONY

Copy link
Copy Markdown
Author

@mircea-c mircea-c Aug 6, 2019

Choose a reason for hiding this comment

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

Just missed removing it from the phony. test_sim_multi_seed_short accomplishes the same thing.

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

Reviewers

@rigelrozanski rigelrozanski Awaiting requested review from rigelrozanski

4 more reviewers

@alexanderbez alexanderbez alexanderbez left review comments

@fedekunze fedekunze fedekunze approved these changes

@alessio alessio alessio approved these changes

@jackzampolin jackzampolin jackzampolin approved these changes

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.

Update CircleCI config

5 participants

@mircea-c @alessio @alexanderbez @jackzampolin @fedekunze