Skip to content

Conversation

@faminours
Copy link
Contributor

Standardize all commands' doc

jnquintin and others added 30 commits August 7, 2017 13:16
Fixes errors in RoutingAlgorithm instanciations :

    $ bxiarchive-addrts ...
    [C] bxiarchive-a Uncaught Exception - exiting thread
    [C] bxiarchive-a TypeError: __init__() takes exactly 2 arguments (1 given)
Simplify Wrapper __init__ super call
add --version option in bxiparserconf addargs
Copy link
Member

@jnquintin jnquintin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we valid that?

@@ -1,14 +1,14 @@
#!/usr/bin/env python
#!/usr/bin/env python2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you try under python3?
is it possible to correct the behaviour if it's not working?

@@ -1,14 +1,14 @@
#!/usr/bin/env python
#!/usr/bin/env python2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

# Please contact Bull S. A. S. for details about its license.
###############################################################################
"""
@author BXIHL <bxihl@atos.net>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we remove the authors?
You could add a contact address but the author is the author...

@@ -1,15 +1,13 @@
#!/usr/bin/env python
#!/usr/bin/env python2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same does it not work under python3?

Please contact Bull SAS for details about its license.\n
Bull - Rue Jean Jaurès - B.P. 68 - 78340 Les Clayes-sous-Bois
@namespace bxilog-console Monitoring console of a bxilog process
@author BXIHL <bxihl@atos.net>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idem author. I don't know any bxihl guy.

@@ -1,14 +1,12 @@
#!/usr/bin/env python
#!/usr/bin/env python2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again python3?

@@ -1,15 +1,13 @@
#!/usr/bin/env python
#!/usr/bin/env python2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again python3?

@trosh
Copy link
Member

trosh commented Nov 21, 2017

About Python2/3: 95% of BXI code is Python2-specific. I have nothing against either making it Python3-specific or Python2/3-compatible, but it's just not something that has been done yet. In the meantime, setting the shebang lines to correspond to the dependency that has been used throughout BXI code makes more sense than leaving the status-quo "unless you can prove that it's not Python3-compatible". Changing the shebang lines would allow having python point to python3 (the default in most-if-not-all Linux distros) without breaking BXI.

About authors=BXIHL: since the actual information of who the author is should be found in Git history, and copyright ownership is Bull's, this top line merely serves as contact information for the client. In practice setting this line manually has caused it to be wrong many times, and the work to keep it correct is just never a priority. Furthermore BXIHL authors tend to stop being maintainers of their code, so the email address shown should be one that lasts longer than that of individual authors.
I'm not against changing @author to @contact, but that is not a Doxygen special command.

@jnquintin
Copy link
Member

Which BXI do you speak of, here on bxibase 95% of the code is compatible. The small part which is not is from the test of posless. Even the packaging has been moved to generate python3 rpm. So yes the code is compatible and should be that way.
I won't speak about other code there since it's on GitHub.

For the author in the file is not used as contact, there is another contact url in the BXIASSERT and in the version file.
author doesn't mean contact.

@trosh
Copy link
Member

trosh commented Nov 22, 2017

Launching bxilog-parser with Python 3:

ModuleNotFoundError: No module named 'Queue'

OK, I can use the commented Python 2/3 compatible block

Traceback (most recent call last):
  File "bxilog-parser", line 81, in <module>
    BLOB2STR_TRANSLATE_TABLE = string.maketrans(_NON_PRINTABLE_STR, _NON_PRINTABLE_SUB)
AttributeError: module 'string' has no attribute 'maketrans'

This can also be made compatible, but it's once again just not something that has been done.
Let's say we use a workaround (str.maketrans instead of string.maketrans):

TypeError: Required argument 'file' (pos 1) not found
<module> at bxilog-parser:453
  main()
main at bxilog-parser:449
  _enqueue_input(args.input, q, args.last)
_enqueue_input at bxilog-parser:266
  input_file = open(name=input_filename, mode='rU', buffering=1)

Once again this can be made compatible ... in the meantime you can't pretend it has been, and the existing status-quo is less correct than the idea it could be fixed. This pull request does not aim to reach compatibility, however removing any amount of status-quo seems to me to be a "free" change that can be added in a commit with close-by changes.

Launching bxiconfig with Python 3:

AttributeError: 'ConfigObj' object has no attribute 'has_key'
<module> at bxiconfig:41
  if conf_file_obj.has_key('modules'):

This is just another example.

Everything BXI has been first written in old-school Python 2, with shebang lines set to the non-explicit "python" in all command scripts. IMO they should all be replaced with "python2", unless explicit effort has been made and verified for Python 2/3 compatibility, in which case it should be explicited after the shebang line. This would differenciate from the existing status quo — had everything been written from the start with compatibility in mind, maybe using "python" with no extra info could make sense, but it's not the case.

Do you want to add compatibility with Python 3? It's just not the same language as Python 2 and compatibility does not come cheap; it also means extra maintainability. My experience of Python 2/3 tells me you're better off writing with only one major version in mind, and being explicit about the version change rather than aiming for general compatibility.

Concerning author versus contact: I am OK with removing author line altogether. I thought BXIHL was fine in the same way that GNU's ls has the word GNU distributed in its documentation, but I admit it feels a bit weird as well being used as "author" information. I'm fine with leaving contact info as it currently is, but I don't think having individual people's @atos.net email address in the scripts is really useful in the long term. Maybe if you want to add your personal address in contact info that could make sense, otherwise letting potential users raise issues in the GitHub page is a practice that has become quite common for most products that rely on GitHub for their source.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants