Ethlint
Ethlint copied to clipboard
duaraghav8
rules to improve
- [ ]
indentation- doesn't lint for indent inside theassemblyblock, also currently doesn't account for the new syntax for defining contract constructor (constructor()- solparse node typeConstructorDeclaration) - [ ]
lbrace- currently doesn't account for the new syntax for defining contract constructor (constructor()- solparse node typeConstructorDeclaration) - [ ]
no-empty-blocksrule completely ignores Function declarations. It should only ignore fallaback functions, other functions should be flagged if they have empty body. - [ ] All whitespace-related rules
NOTE: Changes in rules should pass existing test cases and also add new test cases to test the changes thoroughly
Issue Status: 1. Open 2. Started 3. Submitted 4. Done
This issue now has a funding of 120.0 DAI (120.0 USD @ $1.0/DAI) attached to it.
- If you would like to work on this issue you can 'start work' on the Gitcoin Issue Details page.
- Questions? Checkout Gitcoin Help or the Gitcoin Slack
- $22,450.94 more funded OSS Work available on the Gitcoin Issue Explorer
@duaraghav8 This and https://github.com/duaraghav8/Solium/issues/94 Seem very similar. @ceresstation Can you both figure out how exactly these 2 issues will be done? As there seems to be (a possibility of) redundant work.
@nemaniarjun @gomesalexandre @aerophile there is no overlap. 1 issue exclusively focuses on the fix() function, which is written regardless of how its surrounding code looks like (of course, it needs some info about the solidity code to know what to replace/add/delete).
The other issue focuses on improvement in the sense that the rule itself is written very poorly (for which I am to blame). So it primarily requires refactoring and completing the set of checks the rule must actually be performing. For refactoring, see a few examples of rules that were written more recently using better practices: https://github.com/duaraghav8/Solium/blob/master/lib/rules/function-order.js | https://github.com/duaraghav8/Solium/blob/master/lib/rules/max-len.js | https://github.com/duaraghav8/Solium/blob/master/lib/rules/quotes.js. The older rule implementations need to see similar improvements.
And if a check is not yet present, then you need not worry about implementing a fix for that. eg- if indentation doesn't check indent used by array items declared over multiple lines (at the time when you're developing fixes for that rule), then you need not write a fix for that and your PR will still be accepted.
To simplify things, please work on the bounties in the following order:
- implement
fix()es for current rule implementations - Once these fixes are merged, then work on the rule improvement issue
Sorry for the confusion. Hope I cleared things now? Please let me know if there is any further clarification you need.
@writeprovidence with the previous comment, do you have the information and tools you need to solve this issue? :)
@writeprovidence just a friendly check-in here 😀 have you had a chance to take a look at this issue?
still trying to figure out something
On Thu, Jul 26, 2018 at 8:14 PM, mkosowsk [email protected] wrote:
@writeprovidence https://github.com/writeprovidence just a friendly check-in here 😀 have you had a chance to take a look at this issue?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/duaraghav8/Solium/issues/169#issuecomment-408221269, or mute the thread https://github.com/notifications/unsubscribe-auth/Ad6C1ZlL4u769asO01Xyo_V1bBRp6AsGks5uKiMygaJpZM4RpC_X .
@writeprovidence let me know if it's something you aren't able to figure out from the docs
ok
On Sat, Jul 28, 2018 at 6:47 AM, Raghav Dua [email protected] wrote:
@writeprovidence https://github.com/writeprovidence let me know if it's something you aren't able to figure out from the docs
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/duaraghav8/Solium/issues/169#issuecomment-408587408, or mute the thread https://github.com/notifications/unsubscribe-auth/Ad6C1cDXnupNKqnGkQNHudChsCxrAig0ks5uLAkMgaJpZM4RpC_X .
Issue Status: 1. Open 2. Started 3. Submitted 4. Done
Work has been started.
These users each claimed they can complete the work by 1 year, 5 months ago. Please review their action plans below:
1) mridulnagpal has started work.
If this is still open I would love to work on this issue 2) dhamaniasad has started work.
If there's still interest in fixing this issue I'd like to take it up. 3) raininja has started work.
I plan to identify the issues described in the description and correct them as requested. 4) vporton has started work.
I plan to identify the issues described in the description and correct them as requested.
Learn more on the Gitcoin Issue Details page.
@duaraghav8 Can you tell me which all rules are still open to work on? I finally have some time to do this! :smile:
Hey @nemaniarjun, sorry for replying so late. All rules are still open. There was only 1 PR opened a while ago but I it hasn't been worked on for a while so I'll favour another PR over it.
@duaraghav8 is there a special set of rules to indent for assembly blocks? or should they all be indented one level deeper at the same level in the assembly block? Also, what should be done for the whitespace rules?
Thanks!
ah, sorry I didn't write a proper description for whitespace rules. currently, they're written very poorly - hacks put together through regular expressions. what is needed primarily is the refactoring of whitespace rules such that they pass all existing tests, use ES6 and become much more maintainable (ie use good JS practices).
there's no separate rule for indenting assembly, all indentation-related checks are in a single rule file.
@duaraghav8 Thanks I'll take a look at refactoring the whitespace rule for the tests and try some ES6 best practices that I know of.
is there any specific assembly style indentation/style guide you would like to follow? or should all inline assembly be on the same column, unless there is a label to jump to or an additional block in the assembly, and that's it; an example of this is here: https://github.com/androlo/solidity-workshop/blob/master/tutorials/2016-04-04-solidity-inline-assembly-I.md
All on same column + indentation for labels or blocks (because that is more natural and intuitive for end user)
Hello duaraghav8! This is Alisa from Gitcoin. Is this issue still active?
Hey Alisa, The issue on the project is still active. I'm unsure of its status on Gitcoin. If it is open on Gitcoin, feel free to close it as I don't see anyone taking this up.
I can give this a whirl @duaraghav8, I'm already familiar with the codebase, what say you?
@raininja I'm no longer sure whether the bounties are still valid or not. I can confirm that the issue is still active @PixelantDesign but not sure if the bounty still applies or not.
Hi @raininja you are welcome to take this one on if you have time, I'll even add a tip. Just approved you.
Issue Status: 1. Open 2. Started 3. Submitted 4. Done
Work for 120.0 DAI (120.0 USD @ $1.0/DAI) has been submitted by:
@ceresstation please take a look at the submitted work:
- PR by @vporton
- Learn more on the Gitcoin Issue Details page
- Want to chip in? Add your own contribution here.
- Questions? Checkout Gitcoin Help or the Gitcoin Slack
- $80,075.60 more funded OSS Work available on the Gitcoin Issue Explorer
Issue Status: 1. Open 2. Started 3. Submitted 4. Done
The funding of 120.0 DAI (120.0 USD @ $1.0/DAI) attached to this issue has been approved & issued to @vporton.
- Learn more on the Gitcoin Issue Details page
- Questions? Checkout Gitcoin Help or the Gitcoin Slack
- $75,165.74 more funded OSS Work available on the Gitcoin Issue Explorer