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.

Sign up for GitHub

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

Jump to bottom

Add persistent others to dock #928

Merged
merged 1 commit into from May 14, 2024
Merged

Add persistent others to dock #928

merged 1 commit into from May 14, 2024
+44 −0

Conversation

rmgpinto
Copy link
Contributor

@rmgpinto rmgpinto commented Apr 8, 2024 •
edited

Add persistent others to dock. Closes #927.

@rmgpinto rmgpinto mentioned this pull request Apr 8, 2024
Closed
@rmgpinto
Copy link
Contributor Author

rmgpinto commented Apr 8, 2024

@Enzime @Samasaur1

Samasaur1
Samasaur1 suggested changes Apr 9, 2024
View reviewed changes
Copy link
Contributor

@Samasaur1 Samasaur1 left a comment

Choose a reason for hiding this comment

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

The actual change here looks good to me, but I'd like it if you could avoid making unrelated formatting-only changes in the same PR

@rmgpinto
Copy link
Contributor Author

@Samasaur1 I didn't notice, it's my linter and auto format on save. I've reverted the formatting changes.

@rmgpinto
Copy link
Contributor Author

@Samasaur1 I can't seem to understand why the tests fail. Are you able to help?

@Samasaur1
Copy link
Contributor

Okay I manually checked the failing test on unstable, and it's only failing because of a missing newline. Seems like a false alarm; let me take a closer look and determine whether it's better to change the test or change the implementation back

@Samasaur1
Copy link
Contributor

Ah, okay, you need another newline in the test file

@rmgpinto
Copy link
Contributor Author

Thanks @Samasaur1, tests passed.

@Enzime
Copy link
Collaborator

Enzime commented May 13, 2024

Can you rebase the PR?

@rmgpinto
Copy link
Contributor Author

rmgpinto commented May 13, 2024 •
edited

done @Enzime.

@Enzime
Copy link
Collaborator

Enzime commented May 13, 2024

Can you rebase your PR on top of the latest master and squash your commits? Ideally your branch shouldn't contain the fmt commits and should only contain your commits

@rmgpinto rmgpinto force-pushed the persistent-others branch 2 times, most recently from ae6dd46 to e88183a Compare May 13, 2024 12:46
@rmgpinto
Copy link
Contributor Author

should be ok now @Enzime

@rmgpinto
Copy link
Contributor Author

The tests seem to be flaky, sometimes that check passes, others it doesn't.

Enzime
Enzime approved these changes May 14, 2024
View reviewed changes
Copy link
Collaborator

@Enzime Enzime left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks for the contribution

@Enzime Enzime merged commit de8b0d6 into LnL7:master May 14, 2024
6 checks passed
@thegleich
Copy link
Contributor

This introduces a regression (warning) by using lib.mdDoc

mdDoc usage has been been removed in #932

@rmgpinto
Copy link
Contributor Author

Fixing in #950

@rmgpinto rmgpinto mentioned this pull request May 15, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Samasaur1 Samasaur1 Samasaur1 requested changes

@Enzime Enzime Enzime approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Feature request: add com.apple.dock.persistent-others
4 participants
@rmgpinto @Samasaur1 @Enzime @thegleich