Skip to content
This repository has been archived by the owner on Dec 31, 2024. It is now read-only.

Renaming LiquidPledgingBase: idxDelegate vs idDelegate & projectID #47

Open
GriffGreen opened this issue Nov 22, 2017 · 4 comments
Open
Assignees

Comments

@GriffGreen
Copy link
Member

  1. projectId --> idProject
    projectId is listed just a couple times where as idProject is listed a bunch, please change!

  2. idxDelegate vs idDelegate

This is sooooo confusing, and I am fairly certain it is used inconsistently throughout LiquidPledging base

If we follow the pattern laid out with idProject and idGiver, idDelegate should refer to the id number in the admins array... but we call that idxDelegate in addDelegate(), updateDelegate(), getDelegateIdx() (which has a messed up name cause it has the idxDelegate as a parameter!)

Some times idxDelegate is used the way i would expect it to be used, as the index number for its authority in the delegation chain... this happens in getPledgeDelegate() and in the function namegetDelegateIdx();-)

I would love to hear comments on this, but i see 2 proposals:

My preferred (and easier):
Check all the smart contracts and tests for consistent use of:
idDelegate: the id number in the admin array
idxDelegate: the index number in delegateChain array, different for each specific Pledge

Option 2:

Check all the smart contracts and tests for consistent use of:
idDelegate: the index number in delegateChain array, different for each specific Pledge
idxDelegate: the id number in the admin array

And change:
idGiver -> idxGiver
idProject -> idxProject

@GriffGreen GriffGreen assigned GriffGreen and jbaylina and unassigned GriffGreen Nov 22, 2017
@Quazia
Copy link
Collaborator

Quazia commented Nov 25, 2017

Would it really be that big of a deal to just do delegateIndex or something like that?

@ewingrj
Copy link
Contributor

ewingrj commented Dec 2, 2017

Option 1 is how it should be. We just need to reconcile any inconsistencies.

@ewingrj
Copy link
Contributor

ewingrj commented Dec 2, 2017

Would it really be that big of a deal to just do delegateIndex or something like that?

No, but that's not any more informative as both refer to indexs. 1 in the admins array and the other in the delegationChain array for an individual pledge

@ewingrj
Copy link
Contributor

ewingrj commented Dec 5, 2017

I think this is done, just need to verify the following

Check all the smart contracts and tests for consistent use of:
idDelegate: the id number in the admin array
idxDelegate: the index number in delegateChain array, different for each specific Pledge

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants