Replaced USE_SOLARIS definition with __sun. #4

Merged
MicheleC merged 1 commits from feat/solaris into master 4 months ago
Owner

As per title.
Removal of USE_SOLARIS from autotools files will be done at a later stage, after replacement of USE_SOLARIS has been done throughout all TDE code.

@obache could you check/test whether this PR looks good? For following packages I can do similar replacements but for the first one I would like to have your feedback (relates to TDE/tde#74).
What about the __solaris__ define? Is that ok or should that be removed too?

As per title. Removal of USE_SOLARIS from autotools files will be done at a later stage, after replacement of USE_SOLARIS has been done throughout all TDE code. @obache could you check/test whether this PR looks good? For following packages I can do similar replacements but for the first one I would like to have your feedback (relates to TDE/tde#74). What about the ```__solaris__``` define? Is that ok or should that be removed too?
MicheleC added 1 commit 4 months ago
863a27ccc0
Replaced USE_SOLARIS definition with __sun.
MicheleC requested review from obache 4 months ago
MicheleC requested review from SlavekB 4 months ago
Collaborator

I don't know about __solaris__, but neither GCC nor Oracle Solaris Studio cc define the macro now.

For smb4k/core/smb4kmounter.cpp, It should be replaced with __sun too, because it is used below alone, not with USE_SOLARIS.

I don't know about `__solaris__`, but neither GCC nor Oracle Solaris Studio cc define the macro now. For smb4k/core/smb4kmounter.cpp, It should be replaced with `__sun` too, because it is used below alone, not with `USE_SOLARIS`.
Poster
Owner

I don't know about __solaris__, but neither GCC nor Oracle Solaris Studio cc define the macro now.

For smb4k/core/smb4kmounter.cpp, It should be replaced with __sun too, because it is used below alone, not with USE_SOLARIS.

Ok, thanks for the feedback. I will update the PR later tonight and push an update.

> I don't know about `__solaris__`, but neither GCC nor Oracle Solaris Studio cc define the macro now. > > For smb4k/core/smb4kmounter.cpp, It should be replaced with `__sun` too, because it is used below alone, not with `USE_SOLARIS`. Ok, thanks for the feedback. I will update the PR later tonight and push an update.
MicheleC force-pushed feat/solaris from 863a27ccc0 to 497297648c 4 months ago
Poster
Owner

@obache I pushed an updated version of the commit. Please let me know what you think.

@obache I pushed an updated version of the commit. Please let me know what you think.
SlavekB approved these changes 4 months ago
SlavekB left a comment
Owner

Although I don't have the opportunity to test it on Solaris, it looks right.

Although I don't have the opportunity to test it on Solaris, it looks right.
Poster
Owner

Although I don't have the opportunity to test it on Solaris, it looks right.

Thanks @SlavekB, I will wait for Obata-san to feedback before merging.

> Although I don't have the opportunity to test it on Solaris, it looks right. Thanks @SlavekB, I will wait for Obata-san to feedback before merging.
obache refused to review 4 months ago
Collaborator

looks good.
(accidentally refused to review)

looks good. (accidentally refused to review)
MicheleC merged commit 497297648c into master 4 months ago
MicheleC deleted branch feat/solaris 4 months ago
Poster
Owner

looks good.
(accidentally refused to review)

Thanks Obata-san. I will made similar changes for the other modules in coming days without creating PRs, unless I need to ask for some reasons.

> looks good. > (accidentally refused to review) Thanks Obata-san. I will made similar changes for the other modules in coming days without creating PRs, unless I need to ask for some reasons.
MicheleC added this to the R14.0.12 release milestone 4 months ago

Reviewers

SlavekB approved these changes 4 months ago
The pull request has been merged as 497297648c.
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
3 Participants
Notifications
Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

Loading…
There is no content yet.