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

hdrop: 0.1.3 -> 0.2.1, Update readme #73

Merged
merged 5 commits into from
Nov 16, 2023
Merged

hdrop: 0.1.3 -> 0.2.1, Update readme #73

merged 5 commits into from
Nov 16, 2023

Conversation

Schweber
Copy link
Contributor

No description provided.

Copy link
Member

@fufexan fufexan left a comment

Choose a reason for hiding this comment

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

Looks good, but I think it would be better to have all the documentation inside a manpage.

@Schweber
Copy link
Contributor Author

Ok, i added it and reduced the help text. I have a question:

On nixpkgs, grimblast has got

  strictDeps = true;

  nativeBuildInputs = [
    makeWrapper
    scdoc
  ];

On hyprwm/contrib it has got

  nativeBuildInputs = [
    makeWrapper
  ];

  buildInputs = [
    scdoc
  ];

What's the difference / which one is better and why?

@fufexan
Copy link
Member

fufexan commented Nov 16, 2023

I think the one in Nixpkgs is idiomatic, as nativeBuildInputs are dependencies which are needed for compilation/package building. buildInputs are generally runtime dependencies. I'll do a refactor sometime.

@Schweber
Copy link
Contributor Author

I see, thanks. Is strictDeps = true; beneficial or can it be left out?

hdrop/hdrop Show resolved Hide resolved
hdrop/hdrop Outdated Show resolved Hide resolved
@Schweber Schweber changed the title hdrop: 0.1.3 -> 0.2.0, Update readme hdrop: 0.1.3 -> 0.2.1, Update readme Nov 16, 2023
@fufexan
Copy link
Member

fufexan commented Nov 16, 2023

Ok, looks good. Only one more thing, can you rename the commits in the style hdrop: <action>? Unless you want me to squash them.

@Schweber
Copy link
Contributor Author

Please squash them, I'll do it correctly the next time.

If i now start doing force pushes or something like that i'll screw it up because i'm just learning Github etc.

@fufexan
Copy link
Member

fufexan commented Nov 16, 2023

I've rebased the commits and squashed a few. Looks good?

@Schweber
Copy link
Contributor Author

To be honest, i don't understand that part yet. But if you're satisfied, then so am i ;)

Copy link
Member

@fufexan fufexan left a comment

Choose a reason for hiding this comment

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

Alright, LGTM. Thanks :)

@fufexan fufexan merged commit 3eaa1ed into hyprwm:main Nov 16, 2023
7 checks passed
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.

2 participants