everypolitician / commons-builder

Build scripts for Democratic Commons repositories
MIT License
1 stars 0 forks source link

Include all legislatures in popolo output whether or not they have memberships #101

Closed crowbot closed 5 years ago

crowbot commented 5 years ago

Fixes #100

codecov[bot] commented 5 years ago

Codecov Report

Merging #101 into master will decrease coverage by 0.01%. The diff coverage is 97.95%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #101      +/-   ##
==========================================
- Coverage    94.6%   94.58%   -0.02%     
==========================================
  Files          35       36       +1     
  Lines         834      850      +16     
==========================================
+ Hits          789      804      +15     
- Misses         45       46       +1
Impacted Files Coverage Δ
test/commons/builder/membership_data_test.rb 100% <ø> (ø) :arrow_up:
lib/commons/builder/membership_data.rb 100% <100%> (ø) :arrow_up:
lib/commons/builder/legislature.rb 95.34% <100%> (+0.9%) :arrow_up:
test/commons/builder/executive_test.rb 100% <100%> (ø) :arrow_up:
lib/commons/builder/executive.rb 94.73% <100%> (+0.79%) :arrow_up:
test/commons/builder/legislative_index_test.rb 100% <100%> (ø) :arrow_up:
test/commons/builder/legislature_test.rb 91.66% <91.66%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 8dbcff0...9018216. Read the comment docs.

alexdutton commented 5 years ago

Looks good, though a couple of minor things:

As you say, it would make sense to do all the legislature/executive names in one go. We could do this by doing labels properly in the executive query, but I'd be happy to sort this out later.

crowbot commented 5 years ago
  • Do we need the seat count changes in this PR? I think we were moving towards term-based seat counts and removing the ?org_seat_count from the legislative query. This is another of my half-finished things though — I think we merged it (#78) and then reverted it (#90) because it caused queries to time out.

This PR completely moves the creation of popolo-json regarding legislatures to the Legislatureobject, from the MembershipData object - so I think we need to move the seat counts too (they're included in the final build). We could do the term-based seat count from here in a later PR?

alexdutton commented 5 years ago

Ah, yep. I hadn't spotted that the seat counts were in there before, so it all makes sense now. I'll look at the term-based seat counts thing again soon