search

davmac314 / dinit

Service monitoring / "init" system
Apache License 2.0
615 stars 49 forks source link

meson: Some minor fixes/enhancements #270

Closed mobin-2008 closed 10 months ago

mobin-2008 commented 10 months ago

Hi. After #263 (I'm waiting for @davmac314 review) It's time to post some minor fixes/patches for meson. first commit is bugfix and other are enhancements. Each commit has its title (and its description about utmp related commit) for more info.

Regards

Signed-off-by: Mobin "Hojjat" Aydinfar <mobin@mobintestserver.ir>

mobin-2008 commented 10 months ago

I removed my request for review because maybe you want to close #263 first. I opened this PR for CI checks mainly (CI checks in forks is painful!).

davmac314 commented 10 months ago

I opened this PR for CI checks mainly (CI checks in forks is painful!).

There's generally no need to run full CI until a PR is ready. You can test it locally until then. I would prefer you don't open PR until you believe it's ready (if it fails CI, it's fine to fix it then). No need to request review, I already get a notification that the PR is opened, the request for review is just another notification.

Pushing changes to PR once it's opened is best avoided because I may have already started reviewing. Then I have to start again or look separately at the changes between the different revisions. So please try to keep that to a minimum - get everything ready, check it over carefully, and then do one push when it's ready.

mobin-2008 commented 10 months ago

I opened this PR for CI checks mainly (CI checks in forks is painful!).

There's generally no need to run full CI until a PR is ready. You can test it locally until then. I would prefer you don't open PR until you believe it's ready (if it fails CI, it's fine to fix it then). No need to request review, I already get a notification that the PR is opened, the request for review is just another notification.

Pushing changes to PR once it's opened is best avoided because I may have already started reviewing. Then I have to start again or look separately at the changes between the different revisions. So please try to keep that to a minimum - get everything ready, check it over carefully, and then do one push when it's ready.

I just reworded commit message of bugfix to include what error got fixed. I didn't change code.

davmac314 commented 10 months ago

I just reworded commit message of bugfix to include what error got fixed. I didn't change code.

I know, it's fine. I think I misunderstood what you meant when you said you opened it for CI checks mainly.

Anyway, it's fine to open multiple PRs, I will get to them when I have time. This one was easy to review so I did so.

mobin-2008 commented 10 months ago

Hmm "Merging is blocked The base branch does not allow updates. Learn more about protected branches."

davmac314 commented 10 months ago

Seems I locked the branch accidentally, I unlocked it now.

mobin-2008 commented 10 months ago

~Looks like it was a bug with github web,~ I merged with github mobile