New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Give birth to the YANGBear #1428

Open

userzimmermann wants to merge 1 commit into master from zimmermann/yang

Member

userzimmermann commented Feb 13, 2017 •

edited

Loading

As new member of https://github.com/OpenNetworkingFoundation I obviously want to add coala code analysis to their repositories :) One of the most important languages there is YANG for data modeling. The new standard for defining network management software functions in future telecommunication infrastructures... But there is no YANGBear yet. Well, here it is...

It uses https://github.com/mbj4668/pyang for linting .yang model files.

It only needs a way to configure if also warnings should be complained about or not, and of course more comprehensive tests, to get out of WIP!

Closes #2032

userzimmermann added area/lintbears importance/high labels

userzimmermann self-assigned this

userzimmermann requested review from sils and Makman2

February 13, 2017 20:40

gitmate-bot added process/wip size/M labels

gitmate-bot reviewed

View reviewed changes

tests/yang/YANGBearTest.py Outdated

+              VALID = requests.get(
+                  "https://raw.githubusercontent.com/OpenNetworkingFoundation/CENTENNIAL"
+                  "/master/models/yang/core-model.yang"

Collaborator

gitmate-bot Feb 13, 2017

You do not use the preferred quotation marks.

QuotesBear, severity NORMAL, section python.

The issue can be fixed by applying the following patch:

--- a/tests/yang/YANGBearTest.py
+++ b/tests/yang/YANGBearTest.py
@@ -6,7 +6,7 @@
 
 VALID = requests.get(
     "https://raw.githubusercontent.com/OpenNetworkingFoundation/CENTENNIAL"
-    "/master/models/yang/core-model.yang"
+    '/master/models/yang/core-model.yang'
 ).text

gitmate-bot reviewed

View reviewed changes

tests/yang/YANGBearTest.py Outdated



		VALID = requests.get(
		"https://raw.githubusercontent.com/OpenNetworkingFoundation/CENTENNIAL"

Collaborator

gitmate-bot Feb 13, 2017

You do not use the preferred quotation marks.

QuotesBear, severity NORMAL, section python.

The issue can be fixed by applying the following patch:

--- a/tests/yang/YANGBearTest.py
+++ b/tests/yang/YANGBearTest.py
@@ -5,7 +5,7 @@
 
 
 VALID = requests.get(
-    "https://raw.githubusercontent.com/OpenNetworkingFoundation/CENTENNIAL"
+    'https://raw.githubusercontent.com/OpenNetworkingFoundation/CENTENNIAL'
     "/master/models/yang/core-model.yang"
 ).text

userzimmermann force-pushed the zimmermann/yang branch from 2cd99e5 to 782fd8f Compare

February 14, 2017 13:10

userzimmermann mentioned this pull request

WIP: Add .coafile & .travis.yml to lint **.yang & **.js Open-Network-Models-and-Interfaces-ONMI/onmi-iisomi-xhaul-uml-yang#12

Merged

Collaborator

gitmate-bot commented Feb 21, 2017

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

1 similar comment

Collaborator

gitmate-bot commented Feb 21, 2017

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

Member

Makman2 commented Feb 24, 2017

looks good basically, so why did you mark the PR as WIP? :)

Member

Makman2 commented Feb 24, 2017

(sorry btw for not looking at it until now :/ )

Collaborator

gitmate-bot commented Mar 4, 2017

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

3 similar comments

Collaborator

gitmate-bot commented Mar 11, 2017

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

Collaborator

gitmate-bot commented Mar 11, 2017

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

Collaborator

gitmate-bot commented Mar 18, 2017

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

Member Author

userzimmermann commented Mar 19, 2017

@Makman2 I just wanted to add more comprehensive tests and some setting to change the YANG module import search path. But I can also do that in a separate step...

userzimmermann changed the title ~~WIP: Give birth to the YANGBear~~ Give birth to the YANGBear

userzimmermann removed the process/wip label

Member

Makman2 commented Mar 19, 2017

If you think those tests are okay for a first step, then imo it suffices to get a review, and you can add better tests later 👍

Collaborator

gitmate-bot commented Mar 27, 2017

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

userzimmermann force-pushed the zimmermann/yang branch from 782fd8f to 591da67 Compare

March 31, 2017 22:03

gitmate-bot added the process/pending review label

Member Author

userzimmermann commented Mar 31, 2017

@Makman2 So please review! :)

Meanwhile I also added the new SEE_MORE attribute according to #1136 , coala/coala#3978 , and coala/coala#3979

userzimmermann mentioned this pull request

Every linter bear should include a URL to the linker website #1136

Open

23 tasks

Collaborator

gitmate-bot commented Apr 8, 2017

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

jayvdb requested changes

View reviewed changes

Member

jayvdb left a comment

Quite good. I'm a bit worried lxml might become a dependency, but hopefully not before we're made python deps optional.

tests/yang/YANGBearTest.py Outdated

		from coalib.testing.LocalBearTestHelper import verify_local_bear


		VALID = requests.get(

Member

jayvdb Apr 8, 2017

Please store this in our repo, or create simple tests if that file cant be licenced as MIT.

Member Author

userzimmermann Apr 8, 2017

@jayvdb Or I will create a YANG model for coala itself :D Hmm... Let's see!

Member

sils Jun 30, 2017

still open to address

tests/yang/YANGBearTest.py Outdated



		# several missing module sub-statements
		INVALID = """

Member

jayvdb Apr 8, 2017

There will eventually be more than one invalid file. Please give this variable a useful name.

jayvdb removed the process/pending review label

userzimmermann force-pushed the zimmermann/yang branch from e87a725 to 2548bd7 Compare

August 4, 2017 20:08

gitmate-bot added process/pending review and removed process/wip labels

userzimmermann force-pushed the zimmermann/yang branch from 2548bd7 to 5841c30 Compare

August 5, 2017 10:12

Member Author

userzimmermann commented Aug 5, 2017 •

edited

Loading

So... Only tests for new yang_search_paths setting missing...

BTW @Makman2 Is this a good setting name? That's a serious question ;) And are there also test helpers for such tasks?

Member

Makman2 commented Aug 5, 2017

Looks okay, though what are they used for in YANG? :) A docstring is always good ;)

damngamerz reviewed

View reviewed changes

tests/yang/YANGBearTest.py Outdated

+              module coala-model {
+                namespace "http://coala.io/yang/coala-model";
+                prefix coala;

Member

damngamerz Aug 19, 2017

These string literals looks ugly....breaks line of flow
Use this https://docs.python.org/2/library/textwrap.html#textwrap.dedent
This is not a compulsory change..It's just a suggestion 😉
Rest LGTM :)

Member Author

userzimmermann Sep 5, 2017

@damngamerz It is compulsory! Believe me ;)

Member Author

userzimmermann Sep 7, 2017

@damngamerz Actually it's compulsory to move them to separate .yang files! :)

Member

damngamerz Sep 7, 2017 •

edited

Loading

So Im guessing we wont be requiring those string literals...Much better :) 👍

meetmangukiya reviewed

View reviewed changes

tests/yang/YANGBearTest.py Outdated

		from coalib.testing.LocalBearTestHelper import verify_local_bear


		COALA_MODEL = """

Member

meetmangukiya Aug 28, 2017

capital C :(

Member Author

userzimmermann Sep 5, 2017

@meetmangukiya Shame on me! ;P

gitmate-bot added the status/STALE label

userzimmermann force-pushed the zimmermann/yang branch from 5841c30 to 411142c Compare

September 5, 2017 15:19

gitmate-bot added size/S and removed size/M status/STALE labels

userzimmermann force-pushed the zimmermann/yang branch 3 times, most recently from 4b1ba67 to 4f96f75 Compare

September 7, 2017 17:19


          Give birth to the YANGBear

4c3b6c3

Closes #2032

userzimmermann mentioned this pull request

Give birth to the YANGBear #2032

Open

userzimmermann force-pushed the zimmermann/yang branch from 4f96f75 to 4c3b6c3 Compare

September 7, 2017 18:44

gitmate-bot added the bear proposal label

gitmate-bot added the status/STALE label

shreyans800755 reviewed

View reviewed changes

bears/yang/YANGBear.py

+                      use_stderr=True,
+                      output_format='regex',
+                      output_regex=r'.*:(?P<line>\d+):\s*(?P<message>.*)')
+              class YANGBear:

Contributor

shreyans800755 Oct 30, 2017

Shouldn't this class derive from some base bear class ?

Member

Makman2 Oct 30, 2017

Not with @linter ;)

Member

nemani commented Feb 24, 2018

Hey, tests are failing, should we drop this out of review queue?

Member

nemani commented Feb 24, 2018

@Makman2 @userzimmermann This PR is almost complete, with just the window part needed to be handled.
I say we mark it as WIP unitill @userzimmermann gets time to finish it :D

jayvdb removed bear proposal importance/high labels

gitmate-bot removed the status/STALE label

jayvdb added process/wip and removed process/pending review labels

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

Reviewers

shreyans800755 shreyans800755 left review comments

meetmangukiya meetmangukiya left review comments

damngamerz damngamerz left review comments

gitmate-bot gitmate-bot left review comments

jayvdb jayvdb requested changes

sils sils requested changes

Makman2 Awaiting requested review from Makman2

Labels

area/lintbears process/wip size/S