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

Rebase Boss MVP by @edgarssilva #222

Merged
merged 1 commit into from
Aug 11, 2022
Merged

Rebase Boss MVP by @edgarssilva #222

merged 1 commit into from
Aug 11, 2022

Conversation

zicklag
Copy link
Member

@zicklag zicklag commented Aug 11, 2022

Rebases #200 onto the latest master, with a couple subjective tweaks:

  • Make boss flag in YAML a bool instead of an Option<bool>
  • Impl HasLoadProgress for bool instead of ignoring the boss field
    in the metata struct.

Otherwise this is pretty much all just @edgarssilva's code.

@edgarssilva , you can push to this branch to do anything else you want to do with this before merging, if anything.

Rebases #200 onto the latest master, with a couple subjective tweaks:

- Make boss flag in YAML a `bool` instead of an `Option<bool>
- Impl HasLoadProgress for `bool` instead of ignoring the `boss` field
  in the metata struct.

Co-authored by: Edgar Soares da Silva <70528288+edgarssilva@users.noreply.github.com>
@zicklag zicklag requested a review from edgarssilva August 11, 2022 02:44
@zicklag zicklag mentioned this pull request Aug 11, 2022
Copy link
Collaborator

@edgarssilva edgarssilva left a comment

Choose a reason for hiding this comment

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

Seems great thanks for the help.
Also, the attack logic isn't great and can lead to odd behavior but I didn't worry too much since we are changing it later.

@edgarssilva edgarssilva merged commit 4adbe2d into master Aug 11, 2022
@odecay
Copy link
Collaborator

odecay commented Aug 11, 2022

I think we are supposed to be using bors to merge stuff now, and remember to make the formatting checks pass also

@edgarssilva
Copy link
Collaborator

I think we are supposed to be using bors to merge stuff now, and remember to make the formatting checks pass also

Ho your right sorry, totally went over my head.

@zicklag zicklag deleted the boss-mvp branch August 11, 2022 14:29
@zicklag
Copy link
Member Author

zicklag commented Aug 11, 2022

You can require bors to pass before merging on the branch protection rules, which would prevent this from accidentally happening in the future. I've almost clicked it a couple of times before remembering I was supposed to tell bors to merge it:

image

@edgarssilva
Copy link
Collaborator

You can require bors to pass before merging on the branch protection rules, which would prevent this from accidentally happening in the future. I've almost clicked it a couple of times before remembering I was supposed to tell bors to merge it:

Done, also should I enable "Require a pull request before merging" so that also doesn't happen accidentally.

@zicklag zicklag mentioned this pull request Aug 11, 2022
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