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

Preamble lint to ensure last-call-deadline is not in the past #21

Open
SamWilsn opened this issue Jul 20, 2022 · 11 comments · May be fixed by #120
Open

Preamble lint to ensure last-call-deadline is not in the past #21

SamWilsn opened this issue Jul 20, 2022 · 11 comments · May be fixed by #120
Assignees
Labels
good first issue Good for newcomers ready to implement Feature has reached rough consensus among editors and is ready to be implemented

Comments

@SamWilsn
Copy link
Contributor

No description provided.

@SamWilsn SamWilsn added the good first issue Good for newcomers label Jul 20, 2022
@Pandapip1
Copy link
Member

Or better: at least two weeks into the future (as per EIP-1)

@SamWilsn
Copy link
Contributor Author

Or better: at least two weeks into the future (as per EIP-1)

I don't think we can do that (at least as part of eipw) because someone could open a PR to fix something minor (ex. typo) during the last call period, without having to change the last-call-deadline.

@MicahZoltu
Copy link

Can EIPW have a check that runs conditional on the specific PR/commit it is running against changing a specific line? If so, we would make it so that if (prChangedStatusLineTo('Last Call')) assert lastCallDeadline > now + 2 weeks

@SamWilsn
Copy link
Contributor Author

I'd like to keep anything GitHub related out of eipw (mostly because the Rust GitHub bindings didn't seem as good as the JS ones.) Doing it with pure git might be possible, but would probably rely on git blame, which wouldn't be the most accurate.

Any reason we couldn't implement the more accurate checks in EIP-bot?

@MicahZoltu
Copy link

Any reason we couldn't implement the more accurate checks in EIP-bot?

Our Rust developer is more active than our TypeScript developer. 😬

@SamWilsn SamWilsn added the ready to implement Feature has reached rough consensus among editors and is ready to be implemented label Aug 18, 2022
@apeaircreative
Copy link
Contributor

@SamWilsn I'd like to take on this task

@SamWilsn
Copy link
Contributor Author

By all means!

@apeaircreative
Copy link
Contributor

so i understand that there is no validation in place to ensure that the "last-call-deadline" is set to a date in the future. I've been looking through eip review bot with the merge.ts, process.ts, action.ts, and the files in the rules and searched "last-call-deadline" and other keywords.

yet still haven't found anything validating the "last-call-dealine".

what do you think about the preMergeChanges function in the merge.ts file? Could that be the place to make changes?

@Pandapip1

@Pandapip1
Copy link
Member

I don't maintain eipw. CC @SamWilsn

@KatyaRyazantseva
Copy link

@apeaircreative I guess you can skip the review bot if you have issues with it. I didn't use it at all. Just focus on last-call-dealine implementation.

@apeaircreative
Copy link
Contributor

@SamWilsn @KatyaRyazantseva solved my first issue check out the pull requesr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers ready to implement Feature has reached rough consensus among editors and is ready to be implemented
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants