search

kunaltyagi / nsiqcppstyle

Cpp style checker in python
GNU General Public License v2.0
26 stars 24 forks source link

Converted to Python3 #22

Closed AnisBoubaker closed 4 years ago

kunaltyagi commented 4 years ago

Id' welcome a PR without the cruft

AnisBoubaker commented 4 years ago

Hi,

I did a pull request after minimally adapting the code to work with python3.

Since I did the bare minimum to get the code to compile, I’ve cancelled the PR as I wasn’t sure you’d accept such a low standard. However I am not intending to spend more time on this as the bulk of my work will be on:

Being in a hurry, I can’t invest much with the conversion to Python3. If it’s ok with you, I’ll gladly send you à PR. Otherwise I’ll keep working on my fork to get the job done on time.

Thanks.

— Anis Boubaker, Ph.D.

Le 23 déc. 2019 à 12:32 AM, Kunal Tyagi notifications@github.com a écrit :

 Id' welcome a PR without the cruft

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub, or unsubscribe.

kunaltyagi commented 4 years ago

Hi Anish,

Just a minimal PR is fine. You can do that by using git rebase and then moving around the commits as well as modifying the files added/modified in one. That'll hopefully reduce the effort required to create the PR.

As for new rules and json reporter, they sound great, but perhaps part of 2 smaller PR. I'm sure other people would like to have the json reporter at the very least.

As for i18n, although I like the fact that it'll be more inclusive, I have to see the code before saying anything about it. It doesn't affect the dependencies required, so that's a plus IMO.

I intend to use the tool as part of an architecture for automatic grading of assignments at my uni

This sounds nice. Usually people focus on passing tests and not the contents of the code, so it's really interesting to see that you're focusing on the content as well. What other tools do you intend to use for this?

I need the reports to be in various languages

Maybe adding the language in the json output would be a nice idea too. I've never seen a multi-lingual class so you've got me very interested in the changes you're proposing.

Cheers

AnisBoubaker commented 4 years ago

I just did a PR for Py3 changes. The others will follow.

For i18n, I'll use GNU gettext. It has been around for ages as the standard i18n tool in *nix systems. I couldn't say much about windows support. If i18n is anything you're interested in at all, I think there is no going around adding a new dependency, unless reinventing the wheel and having much less tools/support at our disposal. I'm based in Canada and our Uni is bilingual (english/french) and I'd like to give my students reports in their preferred language.

About the other tools I'm using for automatic grading, there is OCLint, CLang compiler warnings and a testing framework I wrote for semantic analysis. I intend to replace OClint by an AST based analysis tool I'm currently working on (mainly because OCLint is a labyrinthine system that'll be hard for me to update).

Anis

On Mon, Dec 23, 2019 at 1:52 AM Kunal Tyagi notifications@github.com wrote:

Hi Anish,

Just a minimal PR is fine. You can do that by using git rebase and then moving around the commits as well as modifying the files added/modified in one. That'll hopefully reduce the effort required to create the PR.

As for new rules and json reporter, they sound great, but perhaps part of 2 smaller PR. I'm sure other people would like to have the json reporter at the very least.

As for i18n, although I like the fact that it'll be more inclusive, I have to see the code before saying anything about it. It doesn't affect the dependencies required, so that's a plus IMO.

I intend to use the tool as part of an architecture for automatic grading of assignments at my uni

This sounds nice. Usually people focus on passing tests and not the contents of the code, so it's really interesting to see that you're focusing on the content as well. What other tools do you intend to use for this?

I need the reports to be in various languages

Maybe adding the language in the json output would be a nice idea too. I've never seen a multi-lingual class so you've got me very interested in the changes you're proposing.

Cheers

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/kunaltyagi/nsiqcppstyle/pull/22?email_source=notifications&email_token=ALDJYMJTWH4EDTFJH3OFLQ3Q2BN4NA5CNFSM4J6FQWAKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHQL75Y#issuecomment-568377335, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALDJYMPRAOHZMXFGZIGXTU3Q2BN4NANCNFSM4J6FQWAA .

kunaltyagi commented 4 years ago

Regarding OCLint, using clang-tidy (iwht pre-written rules) might be helpful since it is based on the compiler's AST. I don't know if the existing rules are as exhaustive as OCLint (as per the home page)

As for GNU gettext, Python (at least 3) has gettext as a core feature