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

rewrite bootstrapping stages #1327

Closed

Conversation

mightyiam
Copy link
Contributor

No description provided.

Copy link
Member

@camelid camelid left a comment

Choose a reason for hiding this comment

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

I'm not sure if a complete rewrite is a good idea. At the very least, it'll probably be a while until this is reviewed since it's such a big change.

@mightyiam mightyiam force-pushed the bootstrapping-stages-re branch 4 times, most recently from fe8583d to beb68df Compare March 14, 2022 07:27
@mightyiam
Copy link
Contributor Author

Alright, I feel I made it more readable in general. And I don't feel I missed anything. I feel it's a step forward all things considered. I don't feel anything important has been removed.

I'm not sure if a complete rewrite is a good idea. At the very least, it'll probably be a while until this is reviewed since it's such a big change.

It seemed to warrant a rewrite, as I was reading it. So that's what I did.

Here's a suggested workflow for reviewing this:

  1. Read the current. As you do, make a list of key points that it addresses.
  2. Read this version. As you do, check the items that it addresses from the list you previously made.

Each item should be explained at least well enough. That is, if it is relevant at all.

@mightyiam mightyiam marked this pull request as ready for review March 14, 2022 07:35
@JohnTitor JohnTitor added the blocked This PR is blocked waiting for some other PR label Apr 14, 2022
@JohnTitor JohnTitor added waiting-on-review This PR is waiting for a reviewer to verify its content and removed blocked This PR is blocked waiting for some other PR labels May 16, 2022
@JohnTitor
Copy link
Member

We've discussed this on the Zulip, and we're going not to merge this as we don't think this doesn't seem to have much improvement.
If you have any thoughts about this decision, feel free to drop them here.

Thanks for proposing changes anyway!

@JohnTitor JohnTitor closed this Jun 5, 2022
@mightyiam
Copy link
Contributor Author

We've discussed this on the Zulip, and we're going not to merge this as we don't think this doesn't seem to have much improvement. If you have any thoughts about this decision, feel free to drop them here.

Thanks for proposing changes anyway!

It was written carefully. I'm certain that it's a significant improvement.

@JohnTitor
Copy link
Member

It was written carefully. I'm certain that it's a significant improvement.

Could you elaborate on what improvements your changes are bringing? I'm not an English native and it's a bit unclear how these changes improve stuff.

@mightyiam
Copy link
Contributor Author

I just updated this branch yet cannot see that update here. Perhaps re-open?

@JohnTitor
Copy link
Member

JohnTitor commented Jun 7, 2022

I cannot, re-opening as another PR would work.

image

@mightyiam
Copy link
Contributor Author

@JohnTitor #1458

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting-on-review This PR is waiting for a reviewer to verify its content
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants