Move header files consumed by both server and client to special folder#333
Move header files consumed by both server and client to special folder#333code-with-amitk wants to merge 1 commit into
Conversation
|
Can one of the admins verify this patch? |
1 similar comment
|
Can one of the admins verify this patch? |
|
Is there a reason why |
47724c8 to
f5d808d
Compare
|
Removing the "Changes requested" label as @Amitkumar50 updated the patchset. @Amitkumar50 also said that "I don't have permissions" (for changing the label). :-\ |
|
@lslebodn Thanks for comment.. I have removed |
|
Although we may not do it properly, we try our best to keep our files grouped together and alphabetically ordered. Also, please, try to follow our commit template (https://github.com/SSSD/sssd/blob/master/.git-commit-template) when writing the commit message. I'm adding the "Changes Requested" label as per this review. |
|
@Amitkumar50, please, do not merge the branches. Just rebase your patches on top of git master.
With that workflow, merge is not needed and our git history is way cleaner. Please, try to do that and re-submit the patches. Feel free to ping me in case with have issues with the described workflow. |
4b4f9b4 to
3036853
Compare
|
@fidencio Done changes .. |
fidencio
left a comment
fidencio
left a comment
There was a problem hiding this comment.
@Amitkumar50, this patch looks better in this interaction!
Let me ask a few more changes though:
- Commit message:
- Please do not include "This PR ..." in the commit messages. Saying "This patch is about ..." is acceptable though.
- forlder -> folder
- Code:
- Please, change the file named util_safealign.h to safealign.h or shared_safealign.h. The util_ namespace made sense when this file belonged to the util's folder, which is not the case anymore.
| along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
| */ | ||
|
|
||
| #include "providers/ldap/sdap_idmap.h" |
There was a problem hiding this comment.
Please, do not change this include's place. It's not related to the patch.
|
Adding the "Changes Requested". |
3036853 to
3296291
Compare
|
Removing the "Changes Requested" label as @Amitkumar50 updates his patch. |
These are the header files which are used by both client and server: src/util/io.h src/util/murmurhash3.h src/util/util_safealign.h This patch is about moving these header files to special folder (src/shared). It will be easier to identify these headers when looking for them in the src tree. util_safalign.h is renamed as safalign.h because util_ namespace is appropriate when this file belonged to the util's folder which is no longer the case. Resolves: https://pagure.io/SSSD/sssd/issue/1898
|
ACK! |
|
5 participants
There are few header files consumed by both client and server. Movement of these headers to some special forlder (src/shared) is desirable.
Resolves: https://pagure.io/SSSD/sssd/issue/1898