Re: [opensc-devel] a few more trivial patches
Frank, greetings -- On Mon, Dec 10, 2012 at 12:23 AM, Frank Morgner morg...@informatik.hu-berlin.de wrote: Your editor should be intelligent enough to guess the right settings. Why have the editor guess, when it could know the right answer? For vim I recently found this plugin http://www.vim.org/scripts/script.php?script_id=4251 There is a similar feature for emacs (c-guess and friends). I am impressed by their cleverness, but it seems better to specify than to guess (with the chance of guessing wrongly). And it's not just tabs-vs-spaces, but also a human judgement as to whether those tabs are intended to represent 4 spaces or 8. No good answers. I just offered the directory-wide settings for emacs as a way to reduce my pain, and possibly to avoid patch submissions with incorrect indentation. Anyway. If Ludovic doesn't want it around, then that's fine by me; I'll find another way to make sure that I use the correct style. :) Best regards, Anthony Foiani ___ opensc-devel mailing list opensc-devel@lists.opensc-project.org http://www.opensc-project.org/mailman/listinfo/opensc-devel
Re: [opensc-devel] a few more trivial patches
2012/12/10 Anthony Foiani anthony.foi...@gmail.com: Ludovic, greetings -- On Sun, Dec 9, 2012 at 7:19 AM, Ludovic Rousseau ludovic.rouss...@gmail.com wrote: 2012/12/8 Anthony Foiani anthony.foi...@gmail.com: Greetings -- I have two small patches which you might want to consider integrating. (And given that I can't get git to do what I want, you probably want to just cherry-pick these, as I suspect I've completely destroyed my repo history...) You should rebase your patches above OpenSC/OpenSC master. Ok, but pardon my git ignorance: I thought that one should never rebase a tree that will be published and pulled from? Or only if it's published and someone tries to *base a new tree* off of it? That is what I thought also. But it is far easier to review a patch when the history is clean. https://github.com/tkil/OpenSC/commit/0c4a2e0c4063f31bc41c34e45869b9a9e7ca41d7 This uses dir local settings to configure Emacs indentation correctly. I don't think that an Emacs configuration file should be added to the OpenSC source code. Hm. Why not? It would ensure that emacs users have their style set appropriately for this project, and shouldn't affect anyone else in any way. In my own use case, I work on 3-4 projects in the same emacs session, and each one has different indentation settings. dir-local settings seem the easiest way to assign a style per directory (tree). You should keep this change in your own branch. And for my second question of git ignorance: how can I maintain my own branch, when merging upstream into a branch is discouraged? Or do I misunderstand the tone of the log comments when trying to check in such a merge? Or just keep the file.dir-locals.el out of git. I have no objection to add this file. I do not use Emacs myself. I see it can help code quality so unless someone objects I will merge it upstream. Please submit a pull request. https://github.com/tkil/OpenSC/commit/599bd1e6c906af63eb379c866076f98a91654cb2 I spotted an inconsistency in how the option argument pointers were initialized; this fixes it (to make it more consistent). Not a bug but the code would be nicer. For whatever it's worth, my understanding is that uninitialized global variables are actually allocated as a part of program runtime, and are initialized to zero at that point. *Initialized* global variables, however, are stored in the binary itself, even if the initializer is zero. So as a matter of style, it might be better to leave all those pointers uninitialized. (This was a big stink on the linux-kernel mailing list a few years back.) On the other hand, I don't know if this behavior is true across all platforms, and the space/time cost in this case is trivial. Can you create a branch from OpenSC/OpenSC master with only this patch and ask for a Pull Request? I'll try. :) Every time I try to use git for anything fancier than an svn-replacement, I seem to get burned... In this case, it looks like I'll have to fork the OpenSC version (instead of the CardContact version), then branch in my new fork, commit this change, and then request a pull of my new branch on the new fork? (Not complaining about amount of work, just trying to make sure I have the flow correct.) Now merged upstream. Merging a pull request from github adds a merge pull request commit. The history is then not very nice (linear) but I don't know a better way using the github web interface. Thanks -- Dr. Ludovic Rousseau ___ opensc-devel mailing list opensc-devel@lists.opensc-project.org http://www.opensc-project.org/mailman/listinfo/opensc-devel
Re: [opensc-devel] a few more trivial patches
Anthony Foiani anthony.foi...@gmail.com writes: Ludovic, greetings -- On Sun, Dec 9, 2012 at 7:19 AM, Ludovic Rousseau ludovic.rouss...@gmail.com wrote: 2012/12/8 Anthony Foiani anthony.foi...@gmail.com: Greetings -- I have two small patches which you might want to consider integrating. (And given that I can't get git to do what I want, you probably want to just cherry-pick these, as I suspect I've completely destroyed my repo history...) You should rebase your patches above OpenSC/OpenSC master. Ok, but pardon my git ignorance: I thought that one should never rebase a tree that will be published and pulled from? Or only if it's published and someone tries to *base a new tree* off of it? This is somewhat controversial, but from my experience in both open source and large private projects, the key issue is not to rebase branches that other people have made commits on top of, or merged into other branches. I find that it's helpful to rebase branches proposed for merging. The point for me is not so much to have them based off recent master to avoid a merge commit, but to produce the clean series of commits that would have appeared had there been no mistakes. Achieving the goal of not rebasing branches others have derived commits From can be accomplished by not rebasing published branches or having an understanding within the project that branches should not be cross-merged, so that rebasing them is still safe. Even if multitple people commit to a branch, with a little coordination this is not a big deal. pgptpvdzhpga8.pgp Description: PGP signature ___ opensc-devel mailing list opensc-devel@lists.opensc-project.org http://www.opensc-project.org/mailman/listinfo/opensc-devel
Re: [opensc-devel] a few more trivial patches
Le 10/12/2012 17:10, Greg Troxel a écrit : Anthony Foiani anthony.foi...@gmail.com writes: Ludovic, greetings -- On Sun, Dec 9, 2012 at 7:19 AM, Ludovic Rousseau ludovic.rouss...@gmail.com wrote: 2012/12/8 Anthony Foiani anthony.foi...@gmail.com: Greetings -- I have two small patches which you might want to consider integrating. (And given that I can't get git to do what I want, you probably want to just cherry-pick these, as I suspect I've completely destroyed my repo history...) You should rebase your patches above OpenSC/OpenSC master. Ok, but pardon my git ignorance: I thought that one should never rebase a tree that will be published and pulled from? Or only if it's published and someone tries to *base a new tree* off of it? This is somewhat controversial, but from my experience in both open source and large private projects, the key issue is not to rebase branches that other people have made commits on top of, or merged into other branches. I find that it's helpful to rebase branches proposed for merging. The point for me is not so much to have them based off recent master to avoid a merge commit, but to produce the clean series of commits that would have appeared had there been no mistakes. Achieving the goal of not rebasing branches others have derived commits From can be accomplished by not rebasing published branches or having an understanding within the project that branches should not be cross-merged, so that rebasing them is still safe. Even if multitple people commit to a branch, with a little coordination this is not a big deal. I also vote for rebase of the feature branch before merging it to the master branch. If this procedure seems annoying, then use cherry-pick, especially when it's going about the minor changes. If no objections, I will rebase two last commits of the current 'master'. Kind regards, Viktor. ___ opensc-devel mailing list opensc-devel@lists.opensc-project.org http://www.opensc-project.org/mailman/listinfo/opensc-devel
Re: [opensc-devel] a few more trivial patches
Ludovic Rousseau wrote: Merging a pull request from github adds a merge pull request commit. The history is then not very nice (linear) but I don't know a better way using the github web interface. It isn't neccessary to use the github web interface just because github is used to host the repository. It works just as well to pull from any remotes and create fast-forward merges. But of course that will then work around the pull request workflow that github enforces. Greg Troxel wrote: You should rebase your patches above OpenSC/OpenSC master. Ok, but pardon my git ignorance: I thought that one should never rebase a tree that will be published and pulled from? Or only if it's published and someone tries to *base a new tree* off of it? This is somewhat controversial, but from my experience in both open source and large private projects, the key issue is not to rebase branches that other people have made commits on top of, or merged into other branches. I disagree that rebasing is controversial. This is just a tool. Rewriting history is never a problem, as long as everyone who is working together wants to understand how repositories work and how easy it is to resync with rewritten branches. Given a local branch foobar that tracks contributor/master here's what to do when the contributor has rewritten her master: git checkout -b orig_contrib_master contributor/master \ git fetch contributor \ git rebase --onto contributor/master orig_contrib_master foobar Clean up after the rebase is done: git branch -D orig_contrib_master Since rebase of the local branch is required, of course it is important that the mechanics of rebase are well understood. I find that it's helpful to rebase branches proposed for merging. There are arguments both ways. I like the look of linear history. On the other hand, depending on the development model, merges may be a more accurate representation of the code history. Viktor Tarasov wrote: I also vote for rebase of the feature branch before merging it to the master branch. Note that any testing and review of the branch should happen *after* that rebase, to avoid a lot of wasted effort. If this procedure seems annoying, then use cherry-pick, especially when it's going about the minor changes. cherry-pick is almost the only thing that rebase does. It's often easier to just use interactive rebase to do the picking. //Peter ___ opensc-devel mailing list opensc-devel@lists.opensc-project.org http://www.opensc-project.org/mailman/listinfo/opensc-devel
Re: [opensc-devel] a few more trivial patches
2012/12/8 Anthony Foiani anthony.foi...@gmail.com: Greetings -- I have two small patches which you might want to consider integrating. (And given that I can't get git to do what I want, you probably want to just cherry-pick these, as I suspect I've completely destroyed my repo history...) You should rebase your patches above OpenSC/OpenSC master. https://github.com/tkil/OpenSC/commit/0c4a2e0c4063f31bc41c34e45869b9a9e7ca41d7 This uses dir local settings to configure Emacs indentation correctly. I don't think that an Emacs configuration file should be added to the OpenSC source code. You should keep this change in your own branch. https://github.com/tkil/OpenSC/commit/599bd1e6c906af63eb379c866076f98a91654cb2 I spotted an inconsistency in how the option argument pointers were initialized; this fixes it (to make it more consistent). Not a bug but the code would be nicer. Can you create a branch from OpenSC/OpenSC master with only this patch and ask for a Pull Request? Bye -- Dr. Ludovic Rousseau ___ opensc-devel mailing list opensc-devel@lists.opensc-project.org http://www.opensc-project.org/mailman/listinfo/opensc-devel
Re: [opensc-devel] a few more trivial patches
Ludovic, greetings -- On Sun, Dec 9, 2012 at 7:19 AM, Ludovic Rousseau ludovic.rouss...@gmail.com wrote: 2012/12/8 Anthony Foiani anthony.foi...@gmail.com: Greetings -- I have two small patches which you might want to consider integrating. (And given that I can't get git to do what I want, you probably want to just cherry-pick these, as I suspect I've completely destroyed my repo history...) You should rebase your patches above OpenSC/OpenSC master. Ok, but pardon my git ignorance: I thought that one should never rebase a tree that will be published and pulled from? Or only if it's published and someone tries to *base a new tree* off of it? https://github.com/tkil/OpenSC/commit/0c4a2e0c4063f31bc41c34e45869b9a9e7ca41d7 This uses dir local settings to configure Emacs indentation correctly. I don't think that an Emacs configuration file should be added to the OpenSC source code. Hm. Why not? It would ensure that emacs users have their style set appropriately for this project, and shouldn't affect anyone else in any way. In my own use case, I work on 3-4 projects in the same emacs session, and each one has different indentation settings. dir-local settings seem the easiest way to assign a style per directory (tree). You should keep this change in your own branch. And for my second question of git ignorance: how can I maintain my own branch, when merging upstream into a branch is discouraged? Or do I misunderstand the tone of the log comments when trying to check in such a merge? https://github.com/tkil/OpenSC/commit/599bd1e6c906af63eb379c866076f98a91654cb2 I spotted an inconsistency in how the option argument pointers were initialized; this fixes it (to make it more consistent). Not a bug but the code would be nicer. For whatever it's worth, my understanding is that uninitialized global variables are actually allocated as a part of program runtime, and are initialized to zero at that point. *Initialized* global variables, however, are stored in the binary itself, even if the initializer is zero. So as a matter of style, it might be better to leave all those pointers uninitialized. (This was a big stink on the linux-kernel mailing list a few years back.) On the other hand, I don't know if this behavior is true across all platforms, and the space/time cost in this case is trivial. Can you create a branch from OpenSC/OpenSC master with only this patch and ask for a Pull Request? I'll try. :) Every time I try to use git for anything fancier than an svn-replacement, I seem to get burned... In this case, it looks like I'll have to fork the OpenSC version (instead of the CardContact version), then branch in my new fork, commit this change, and then request a pull of my new branch on the new fork? (Not complaining about amount of work, just trying to make sure I have the flow correct.) Many thanks, Anthony Foiani ___ opensc-devel mailing list opensc-devel@lists.opensc-project.org http://www.opensc-project.org/mailman/listinfo/opensc-devel
Re: [opensc-devel] a few more trivial patches
Hi! https://github.com/tkil/OpenSC/commit/0c4a2e0c4063f31bc41c34e45869b9a9e7ca41d7 This uses dir local settings to configure Emacs indentation correctly. I don't think that an Emacs configuration file should be added to the OpenSC source code. Hm. Why not? It would ensure that emacs users have their style set appropriately for this project, and shouldn't affect anyone else in any way. In my own use case, I work on 3-4 projects in the same emacs session, and each one has different indentation settings. dir-local settings seem the easiest way to assign a style per directory (tree). Your editor should be intelligent enough to guess the right settings. For vim I recently found this plugin http://www.vim.org/scripts/script.php?script_id=4251 -- Frank Morgner Virtual Smart Card Architecture http://vsmartcard.sourceforge.net OpenPACEhttp://openpace.sourceforge.net IFD Handler for libnfc Devices http://sourceforge.net/projects/ifdnfc pgpBn6TT7CZjQ.pgp Description: PGP signature ___ opensc-devel mailing list opensc-devel@lists.opensc-project.org http://www.opensc-project.org/mailman/listinfo/opensc-devel
