Skip to content

Conversation

@MarshalX
Copy link
Contributor

No description provided.

@MarshalX
Copy link
Contributor Author

@MichalBor in my last commit I use TypedDict. It's available from Python 3.8 but I am a bit confused. The README file of this repository gives me this:
https://github.com/cycodehq-public/cycode-cli/blob/8937ae8f2a5ff656fa7f78d9c6a8d791a144d1d3/README.md?plain=1#L49
but the setup file gives this:
https://github.com/cycodehq-public/cycode-cli/blob/8937ae8f2a5ff656fa7f78d9c6a8d791a144d1d3/setup.py#L46

could you pls tell me what is the lowest version of python that CLI is support?

@MichalBor
Copy link
Contributor

@MichalBor in my last commit I use TypedDict. It's available from Python 3.8 but I am a bit confused. The README file of this repository gives me this:

https://github.com/cycodehq-public/cycode-cli/blob/8937ae8f2a5ff656fa7f78d9c6a8d791a144d1d3/README.md?plain=1#L49

but the setup file gives this:
https://github.com/cycodehq-public/cycode-cli/blob/8937ae8f2a5ff656fa7f78d9c6a8d791a144d1d3/setup.py#L46

could you pls tell me what is the lowest version of python that CLI is support?

ah, honestly I'm not sure. AFAIK we support python 3.7 as well. in general lets stick in avoiding writing a logic that is supported from specific version

@MichalBor
Copy link
Contributor

@artem-fedorov @MarshalX regarding the q above and for better monitoring our users python version usage - lets add python_version param to the "report" scan result API. not urgent, open a ticket to the backlog please

@MarshalX
Copy link
Contributor Author

MarshalX commented Apr 24, 2023

in general lets stick in avoiding writing a logic that is supported from specific version

@MichalBor unfortunately, it is not possible in the Python world( the project uses f-strings already, for example. f strings were added in Python 3.6 and so on. Some features were added in Python 3.7. Well, we need to specify the lowest version of Python for CLI and don't use features from higher versions. All higher versions should work fine

I can do a small research to figure out if there are features in use from Python 3.7 (June 2018). I am 100% sure that at the current state of the project, it doesn't support Python lower than 3.6 (December 2016). Python 3.8 was released in Oct 2019

@MichalBor
Copy link
Contributor

@MarshalX, ok, but lets support 3.7 and above for now, because we don't know what the min python version that used by the users and in the setup.py we declare we support it. regarding python version 3.6 - yep we don't support it. I don't remember exactly why, but we got an error for that specific version and preferred not to fix it since we had not users with this version at that time.

@MarshalX MarshalX requested a review from MichalBor April 24, 2023 15:09
@MarshalX MarshalX force-pushed the CM-22208-format-scan-errors-output-to-json branch from 915c5b0 to bc960ce Compare May 2, 2023 10:42
@MarshalX
Copy link
Contributor Author

MarshalX commented May 2, 2023

@MichalBor I added tests for soft_fail to be 100% sure that everything works fine. I ran tests locally on the main branch and it fully covers _handle_exception. That's all changes that I did after the latest approve. Pls approve again

@MarshalX MarshalX requested a review from MichalBor May 2, 2023 11:59
Copy link
Contributor

@MichalBor MichalBor left a comment

Choose a reason for hiding this comment

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

Well done!

@MarshalX MarshalX force-pushed the CM-22208-format-scan-errors-output-to-json branch from bfdbd3c to ce281a4 Compare May 2, 2023 12:23
@MarshalX
Copy link
Contributor Author

MarshalX commented May 2, 2023

re sign commits due to manipulation from GitHub side

@MarshalX MarshalX merged commit 76f8dee into main May 2, 2023
@MarshalX MarshalX deleted the CM-22208-format-scan-errors-output-to-json branch May 2, 2023 12:27
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.

2 participants