Fix #123. Skip shebang line if present.

[tamere-allo-peter]

This adds support for shebang lines, leaving them unmodified if present, fixing issue #123

[lyz-code]

Hey @tamere-allo-peter congratulations for your first PR!! :)

Your change looks good, but I'm missing some test that makes sure that we don't break this functionality in the future.

It should go in the tests/unit/test_services.py, you can copy one of the existent and tweak it to fit your use case (for example this one).

Also as you can see, the CI failed due to lint errors. If you check the development facilities, you will see that you can use make format to fix most of the linter issues. To see that your tests pass locally before pushing them to the ci, you can use make test.

Any doubt, do not hesitate to ask

[tamere-allo-peter]

@lyz-code Sorry for the formatting, it should be better now with the new commit. As for the unit test I've added it, although it's very different from the existing ones since we don't test the same function. BTW I'm encountering a lot of problems I can't fix right now (dependencies and the like, entirely unrelated to yamlfix) when running make test so I believe it does the job but it's untested on my side.

[lyz-code]

fix_files is a wrapper function to run fix_code for all the files it receives as argument. Therefore, if you test fix_code you're testing that part of fix_files.

Given that the setup of the tests of fix_code is more clear, I'd use it instead.

So my suggestion would be to have in this test:

    source = dedent(
        """\
        #! /this/line/should/be/ignored
        ---
        program: yamlfix
        """
    )

    result = fix_code(source)

    assert result == source
    ```

[tamere-allo-peter]

I respectfully disagree, because my fix for the shebang line problem wasn't implemented in fix_code, but in fix_files. So calling fix_code from the test code to check my patch, which is to fix_files, will always return a failure.

I think it was more straightforward to patch fix_files than fix_code, and I believe it's also the case wrt issue #135

[lyz-code]

I see what you mean. There's another reason why I think it's better to implement this functionality at fix_code level. The program is meant to be used as a library too, and a user may have some string that want's to fix, which doesn't belong from a file. To be able to benefit from your feature, they would need to save that string in a file, and then run fix_files on it.

Please take a moment to think about it, and if you still think it's not worth it, I'll resolve the conversation and merge the PR.

Sorry the PR is taking so long to be resolved, I'm having some busy days and weren't able to review this sooner :(

[tamere-allo-peter]

OK I'll move the fix to fix_code instead of fix_files and modify the test accordingly. I'll try too do this tomorrow.

[lyz-code]

Thank you for the comprehension and the extra work

[lyz-code]

Could you please paste me the errors you're encountering when you run make test?

Did you run make install first?

[tamere-allo-peter]

Mostly fixed the issues
Make install fails for ruyaml (fatal timeout) but works for all the other packages it seems

(env) jerome@OPT17844:~/yamlfix$ make install
python -m pip install -U setuptools pip
Requirement already satisfied: setuptools in ./env/lib/python3.8/site-packages (59.2.0)
Requirement already satisfied: pip in ./env/lib/python3.8/site-packages (21.3.1)
pip install -r requirements-dev.txt
Collecting ruyaml@ git+git://github.com/pycontribs/[email protected]
  Cloning git://github.com/pycontribs/ruyaml (to revision 0.90.0.2) to /tmp/pip-install-q59_hr33/ruyaml_b8e87e17a71f428fa483cc931800f5a6
  Running command git clone --filter=blob:none -q git://github.com/pycontribs/ruyaml /tmp/pip-install-q59_hr33/ruyaml_b8e87e17a71f428fa483cc931800f5a6
  fatal: erreur de lecture: Connexion terminée par expiration du délai d'attente
WARNING: Discarding git+git://github.com/pycontribs/[email protected]. Command errored out with exit status 128: git clone --filter=blob:none -q git://github.com/pycontribs/ruyaml /tmp/pip-install-q59_hr33/ruyaml_b8e87e17a71f428fa483cc931800f5a6 Check the logs for full command output.
Collecting astor==0.8.1
  Using cached astor-0.8.1-py2.py3-none-any.whl (27 kB)
Collecting astpretty==2.1.0
  Using cached astpretty-2.1.0-py2.py3-none-any.whl (5.3 kB)
...
Collecting regex==2021.10.23
  Using cached regex-2021.10.23-cp38-cp38-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (764 kB)
Collecting requests==2.26.0
  Using cached requests-2.26.0-py2.py3-none-any.whl (62 kB)
ERROR: Could not find a version that satisfies the requirement ruyaml (unavailable) (from versions: 0.0.0, 0.19.0a1, 0.19.0, 0.19.2a0, 0.19.2, 0.20.0, 1.0.0a0)
ERROR: No matching distribution found for ruyaml (unavailable)
make: *** [Makefile:8 : install] Erreur 1

[tamere-allo-peter]

I thought it was due to a problem with 2FA but even when adding my PAT to ~/.netrc it doesn't work. Could be a problem with our firewall, although I don't find a problem in the logs I have access to.

The following works fine though, but the download is made through ssh, which was open by our network team

jerome@OPT17844:~$ git clone [email protected]:pycontribs/ruyaml.git
Clonage dans 'ruyaml'...
remote: Enumerating objects: 6192, done.
remote: Counting objects: 100% (900/900), done.
remote: Compressing objects: 100% (437/437), done.
remote: Total 6192 (delta 648), reused 639 (delta 460), pack-reused 5292
Réception d'objets: 100% (6192/6192), 8.53 Mio | 2.76 Mio/s, fait.
Résolution des deltas: 100% (3909/3909), fait.
jerome@OPT17844:~$ 

[lyz-code]

The ruyaml dependency should be solved now with #138 . We had to do it because they haven't published the latest changes](pycontribs/ruyaml#58) on Pypi, and we needed them to fix #120

I've reverted the changes we made, so we can keep on publishing packages.

Can you please update your branch with master and see if you can run make install?

[tamere-allo-peter]

Sorry I think I've done some strange things since I don't really master git... I'll try again in a new PR...