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

Deprecating SBBanPlayer is fine in forks, but then please change your library name #414

Closed
DarkDeviL opened this Issue Mar 11, 2018 · 3 comments

Comments

Projects
None yet
2 participants
@DarkDeviL
Copy link
Contributor

DarkDeviL commented Mar 11, 2018

As the title says, in the commit on March 30, 2017:

Native "SBBanPlayer" was marked as deprecated.

Based on a quick view, it still seems like both the old and new native does the exact same thing, and as such, the deprecation wasn't necessary at all.

Deprecating the "SBBanPlayer" means that there will a lot of issues with warnings for people trying to compile plugins build for the official SourceBans, and essentially removing it completely will lead to issues, as this fork still share the same library name "sourcebans".

As such, a possible solution is to change the sourcebans.inc include file name as well as the contents from "sourcebans" to something like "sourcebanspp" or "sourcebansplusplus"

All existing plugins would of course need to be re-written a little bit to support SourceBans++ then, but it is then possible to support both the official SourceBans, SourceBans++ (even if SBBanPlayer is removed completely), as well as other potential SourceBans "forks", when none of those have the same library name (and natives).

@rumblefrog

This comment has been minimized.

Copy link
Member

rumblefrog commented Mar 11, 2018

@Groruk What you think?

@rumblefrog rumblefrog self-assigned this Mar 11, 2018

@rumblefrog rumblefrog referenced this issue Mar 11, 2018

Merged

Native Fork Namespace #415

5 of 7 tasks complete
@rumblefrog

This comment has been minimized.

Copy link
Member

rumblefrog commented Mar 11, 2018

@DarkDeviL Does #415 suit the solution you are seeking?

@DarkDeviL

This comment has been minimized.

Copy link
Contributor

DarkDeviL commented Mar 12, 2018

Merging #415 will indeed fix this :)

DarkDeviL added a commit to DarkServ/sourcebans-pp that referenced this issue Mar 26, 2018

@DarkDeviL DarkDeviL referenced this issue Mar 26, 2018

Merged

Renaming leftovers from #414 and #415 #429

3 of 7 tasks complete

rumblefrog added a commit that referenced this issue Mar 26, 2018

Merge pull request #429 from DarkServ/v1.x
Renaming leftovers from #414 and #415
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment