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

ECIP-1083: Remove Contract Size Limit #249

Merged
merged 6 commits into from
Mar 4, 2020
Merged

Conversation

BelfordZ
Copy link
Member

@BelfordZ BelfordZ commented Dec 20, 2019

@BelfordZ BelfordZ changed the title fix: remove eip-170 ecip-draft_remove_eip_170 Dec 20, 2019
@BelfordZ BelfordZ changed the title ecip-draft_remove_eip_170 feat: add ecip-draft_remove_eip_170 Dec 20, 2019
@BelfordZ
Copy link
Member Author

bump

@bobsummerwill bobsummerwill changed the title feat: add ecip-draft_remove_eip_170 Remove Contract Size Limit Jan 19, 2020
@soc1c soc1c changed the title Remove Contract Size Limit ECIP-1083: Remove Contract Size Limit Jan 28, 2020
Copy link
Member

@bobsummerwill bobsummerwill left a comment

Choose a reason for hiding this comment

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

Hey Zachary,
I think the only thing holding this up from being merged is that WIP is not a valid status.
Should be "Draft".
Why don't you make that edit, and we get this merged?
It is an important area for consideration.

@ghost
Copy link

ghost commented Feb 4, 2020

@bobsummerwill to my understanding a WIP PR can be merged in that status as an ECIP as per ECIP-1000:

Draft ECIPs which may be in a very early stage may be entered as WIP ECIPs, which means they are a work in progress.

@bobsummerwill
Copy link
Member

Right you are, @tokenhash! I was not aware of that status.
In that case, I see no reason why this should not be merged right now.

@BelfordZ
Copy link
Member Author

BelfordZ commented Feb 4, 2020

needs 1 more editor approval, and I dont think self-approval is appropriate.

bump @whilei

@soc1c
Copy link
Contributor

soc1c commented Feb 5, 2020

WIP shouldn't be merged as the P stands for progress. something in progress cannot be proposed. please proceed to draft stage once you want to propose something and are confident this is a good first draft.

@ghost
Copy link

ghost commented Feb 5, 2020

@soc1c

WIP shouldn't be merged as the P stands for progress. something in progress cannot be proposed. please proceed to draft stage once you want to propose something and are confident this is a good first draft.

Makes sense. If that is the case, then I am going to adjust ECIP-0001 to show WIP can be entered as PRs, but can only advance to merged only if they are stable drafts.

@ghost
Copy link

ghost commented Feb 5, 2020

@bobsummerwill @BelfordZ @soc1c I added/modified this text on ECIP-0001 to reflect the WIP rule:

Draft ECIPs which may be in a very early stage may be entered as WIP pull requests, which means they are a work in progress. However, WIP ECIP pull requests may not be merged into the ECIP repository unless authors feel confident enough about their proposals and moved them to Draft status.

_specs/ecip-1083.md Outdated Show resolved Hide resolved
@bobsummerwill
Copy link
Member

LGTM

Copy link
Member

@bobsummerwill bobsummerwill left a comment

Choose a reason for hiding this comment

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

LGTM

@YazzyYaz
Copy link
Contributor

I can approve it, but first it needs a License unless you specified it somewhere.

_specs/ecip-1083.md Outdated Show resolved Hide resolved
Co-Authored-By: meowsbits <[email protected]>
@BelfordZ
Copy link
Member Author

BelfordZ commented Mar 3, 2020

isnt the default license of apache 2.0 assumed when no license is provided?

@meowsbits
Copy link
Member

meowsbits commented Mar 4, 2020

https://ecips.ethereumclassic.org/ECIPs/ecip-1000#ecip-licensing

Each new ECIP must identify at least one acceptable license in its preamble. ....

status: Draft
type: Standards Track
category: Core
created: 2019-12-20
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
created: 2019-12-20
created: 2019-12-20
license: Apache-2.0

Copy link
Member

@realcodywburns realcodywburns left a comment

Choose a reason for hiding this comment

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

lgtm

@realcodywburns realcodywburns merged commit f525d2a into master Mar 4, 2020
@bobsummerwill
Copy link
Member

"isnt the default license of apache 2.0 assumed when no license is provided?"

There is no default license.

We never added a root license. IP protection is still TODO.

@BelfordZ BelfordZ deleted the fix/remove-eip170 branch April 7, 2020 01:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants