Attempt to integrate XAD into master #65
Conversation
|
I could have a look at this when I'm back from vacation. In any case the maintainer related files under build_aux and po should not be part of this PR.
Can you adjust this, please?
Also: the keymapping parser seem to be not hard-wired to XAD, so that should be moved out to a separate PR which can be reviewed and integrated much more easily.
|
|
FWIW, this repository is not the official GnuCOBOL repository, though Simon is monitoring it and reviewing some of these PRs for official integration. Of course, feel free to use it, but beware OCamlPro is only a contributor and Simon is the true official maintainer. @clademann What is your use-case for GnuCOBOL ? (you can DM me at [email protected]) |
|
Thanks for your note :-) Actually I was suggesting to Christian to use this repo to discuss the PR I hope he finds the time to split the keymapping parser out of this PR to review it separately. |
| @@ -1390,7 +1390,7 @@ scriptversion=2014-01-07.03; # UTC | |||
| # GNU General Public License for more details. | |||
|
|
|||
| # You should have received a copy of the GNU General Public License | |||
| # along with this program. If not, see <https://www.gnu.org/licenses/>. | |||
| # along with this program. If not, see <http://www.gnu.org/licenses/>. | |||
There was a problem hiding this comment.
I saw many of these changes from https to http. Today, isn't it always better to use https rather than http ?
There was a problem hiding this comment.
The file without https are old ones - the PR has a lot of older files checked in.
In general needs to be split into its parts to be able to be reviewed and checked in accordingly, the first one would likely be to add handling of the CONTROL attribute - without the WITH_EXTENDED_ACCDIS parts.
The second one (big, libcob only) would be to add the key mapper.
Potentially the next would be the XAD-additions, but we'd know better when the previous changes would be merged in and working already.
There was a problem hiding this comment.
In any case the maintainer related files under build_aux and po should not be part of this PR.
@clademann please provide an updated PR without those changes
The file without https are old ones - the PR has a lot of older files checked in.
please rebase (in git) in any case, this will fix the "change" to the older file content and also make this PR smaller
In general needs to be split into its parts to be able to be reviewed and checked in accordingly, the first one would likely be to add handling of the
CONTROLattribute - without theWITH_EXTENDED_ACCDISparts.The second one (big, libcob only) would be to add the key mapper.
Potentially the next would be the XAD-additions, but we'd know better when the previous changes would be merged in and working already.
@clademann
In the meantime, the CONTROL attribute was added and also there is a "dynamic" (vararg) function that you can likely use for more attributes - this may mean that you don't need any new exported xad functions at all.
I'd suggest to:
- you'd create a new git branch from gnucobol-3.x (if you prefer, this can also be a svn branch instead) that only adds the keymapper
- I'd review that - and actually this may could land in 3.2 (if it doesn't take weeks)
- as soon as this is "upstream" (in svn in at least one of the mean development branches) - or anytime after depending on your time - we use that version and add the XAD changes "on top" for another PR
What do you think?
Yes, I know it's huge ;-)