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

rfc: 1-animation-slide-side #3

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

rfc: 1-animation-slide-side #3

wants to merge 3 commits into from

Conversation

Ciflire
Copy link

@Ciflire Ciflire commented Jul 16, 2024

I find the implementation wierd with sides.
Decided to use this space to get feedbacks

@vaxerski
Copy link
Member

@NotAShelf is this important enough for an RFC? I think this should just be an enhancement request in hl

@NotAShelf
Copy link
Member

It could be a topic for the RFC if it proposed an entire refactor, but it completely ignores the RFC format.

@NotAShelf
Copy link
Member

"hey can we do this" is not a good RFC topic.

A good RFC

  1. Describes the underlying issue
  2. Convinces the reader that it is in fact an issue and not a nitpick
  3. Proposes changes, and possibly the implementation

@Ciflire
Copy link
Author

Ciflire commented Jul 16, 2024

Could you let me qualify this as a draft and work on it using your feedbacks?

@NotAShelf
Copy link
Member

Do feel free. I'll provide a quick review tomorrow.

@Ciflire Ciflire marked this pull request as draft July 16, 2024 21:06
beside the motivation the rest wasn't modified yet but will be
Some sections are still empty, i don't know what to put it there by
reading the template. But I will fill these up if remarks/questions are
asked.
@Ciflire
Copy link
Author

Ciflire commented Jul 17, 2024

is this important enough for an RFC

I don't know if it is, but it's already implemented and it seems like i have a diverging point of view from the implementation, I thought the best way to settle this was an RFC

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.

3 participants