search

marcoCasamento / Hangfire.Redis.StackExchange

HangFire Redis storage based on original (and now unsupported) Hangfire.Redis but using lovely StackExchange.Redis client
Other
448 stars 108 forks source link

Fixes tests, misc cleanup #120

Closed soenneker closed 1 year ago

soenneker commented 1 year ago

Misc cleanup (no longer preamble for GPL which isn't needed) Tests don't need global lock with tests run in sync

marcoCasamento commented 1 year ago

I've reasoned about the same (now) useless global lock in test, I do agree that is useless if tests run in sequence, I didn't understand however the behavior of the lock with the async Task. I'd like to merge the PR and publish a new version of the package, but I'm dubious about the GPL header. That exact header was requested almost 10 year ago by the original author of this library (@odinserj -author of Hangfire). While I agree that the library has really diverted from his original implementation, I don't understand why that long, a bit annoying preamble isn't needed anymore and I'd prefer asking the original author if he minds.

soenneker commented 1 year ago

Thanks @marcoCasamento, I appreciate the quick review

The GPL preamble per file isn't legally required by the license. The reasoning for it is protection against any file being disconnected from the project.

I think that's an unlikely scenario because this library is completely covered by GPL and the code is so specific to this implementation.

marcoCasamento commented 1 year ago

Ok, I understand. I've just contacted Sergey to have his opinion on this, as I don't really care on that header and I don't want for him to think I'm somehow taking inappropriate credits. Please, let's wait for his response.

soenneker commented 1 year ago

No problem. I can restore the headers if he requires. Thanks @marcoCasamento

odinserj commented 1 year ago

Hi everyone! I don't see any reason why we should remove the LGPL-related headers. They can be folded in different editors, and are for informational reasons for those who try to reuse the code (or reuse the reused one) to be aware of the license.

soenneker commented 1 year ago

Hi everyone! I don't see any reason why we should remove the LGPL-related headers. They can be folded in different editors, and are for informational reasons for those who try to reuse the code (or reuse the reused one) to be aware of the license.

Hi @odinserj, thanks for chiming in. I've added the preambles back.

Yes I agree they can be folded, and it's not a huge burden but I disagree. They're not helpful or informational. GPL in my opinion is not a good license (especially for libraries as it's viral) and these preambles are not likely to accomplish what you're looking to achieve with them.

odinserj commented 1 year ago

GPL in my opinion is not a good license (especially for libraries as it's viral) and these preambles are not likely to accomplish what you're looking to achieve with them.

Please note it's Lesser GPL license that allows commercial usage without disclosing anything. But nevertheless I don't understand what problems will be solved by removing those headers 🤷‍♂️