Skip to content

Question development documentation#9186

Merged
dhalperi merged 3 commits intomasterfrom
ratulm-patch-1
Sep 4, 2024
Merged

Question development documentation#9186
dhalperi merged 3 commits intomasterfrom
ratulm-patch-1

Conversation

@ratulm
Copy link
Copy Markdown
Member

@ratulm ratulm commented Sep 1, 2024

Move over content from https://github.com/batfish/batfish/wiki/Developer-notes. Something is better than nothing.

@ratulm ratulm requested a review from dhalperi September 1, 2024 04:50
@batfish-bot
Copy link
Copy Markdown

This change is Reviewable

@ratulm ratulm requested a review from arifogel September 1, 2024 04:51
@codecov
Copy link
Copy Markdown

codecov bot commented Sep 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.70%. Comparing base (29939d1) to head (4e1327c).
Report is 42 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9186      +/-   ##
==========================================
- Coverage   72.72%   72.70%   -0.02%     
==========================================
  Files        3313     3313              
  Lines      169946   169930      -16     
  Branches    20018    20039      +21     
==========================================
- Hits       123586   123551      -35     
- Misses      37197    37210      +13     
- Partials     9163     9169       +6     

see 6 files with indirect coverage changes

Copy link
Copy Markdown
Member

@dhalperi dhalperi left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @arifogel and @ratulm)


docs/question_development/README.md line 5 at r1 (raw file):

The high-level architecture of questions is as follows.

What is called via pybtafish (bf.q.<foo>) is a JSON template. See example templates inside the subfolders of [questions in the Batfish repository](https://github.com/batfish/batfish/tree/master/questions).

Suggestion:

pybatfish 

docs/question_development/README.md line 5 at r1 (raw file):

The high-level architecture of questions is as follows.

What is called via pybtafish (bf.q.<foo>) is a JSON template. See example templates inside the subfolders of [questions in the Batfish repository](https://github.com/batfish/batfish/tree/master/questions).

Suggestion:

`bf.q.<foo>`

docs/question_development/README.md line 11 at r1 (raw file):

### Adding a new Java question

See an existing question (e.g., `CompareFiltersQuestionPlugin`) for the various classes that are needed, as well as the structure of each class and their relationships among one another.

consider adding a link to the question module


docs/question_development/README.md line 13 at r1 (raw file):

See an existing question (e.g., `CompareFiltersQuestionPlugin`) for the various classes that are needed, as well as the structure of each class and their relationships among one another.

Small but important note:  the getName() method in your subclass of Question must return a String that is all lowercase. Otherwise clients will not be able to find your question when looking for it on the classpath.

and code font throughout?

Suggestion:

`getName()` 

docs/question_development/README.md line 13 at r1 (raw file):

See an existing question (e.g., `CompareFiltersQuestionPlugin`) for the various classes that are needed, as well as the structure of each class and their relationships among one another.

Small but important note:  the getName() method in your subclass of Question must return a String that is all lowercase. Otherwise clients will not be able to find your question when looking for it on the classpath.

do you know more about this? A bunch that I spot checked don't have names that are all lowercase. https://github.com/batfish/batfish/blob/master/projects/question/src/main/java/org/batfish/question/initialization/FileParseStatusQuestion.java#L14

Code quote:

ust return a String that is all lowercase

docs/question_development/README.md line 17 at r1 (raw file):

### Adding a new template

Follow examples in the existing templates.

consider adding a link direct into the templates folder.

Copy link
Copy Markdown
Member Author

@ratulm ratulm left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 files reviewed, all discussions resolved (waiting on @arifogel and @dhalperi)


docs/question_development/README.md line 5 at r1 (raw file):

The high-level architecture of questions is as follows.

What is called via pybtafish (bf.q.<foo>) is a JSON template. See example templates inside the subfolders of [questions in the Batfish repository](https://github.com/batfish/batfish/tree/master/questions).

done

Copy link
Copy Markdown
Member

@dhalperi dhalperi left a comment

Choose a reason for hiding this comment

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

Reviewed all commit messages.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @arifogel and @ratulm)


docs/question_development/README.md line 11 at r2 (raw file):

See existing questions (e.g., [CompareFilters](https://github.com/batfish/batfish/tree/master/projects/question/src/main/java/org/batfish/question/comparefilters)) for example of how to extend these classes. 

NB: the `getName()` method in your subclass of Question must return a String that is unique across all questions. Batfish finds questions using this name. The names are case-insensitive (so, `FooBar` is the same name as `fooBar` and `foobar`).

I bet pybatfish generated names are sourced from it, too.

So fileParseStatus -> bf.q.fileParseStatus.

If you can confirm, consider updating again :p. Sorry.

Code quote:

Batfish finds questions using this name

Copy link
Copy Markdown
Member Author

@ratulm ratulm left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @arifogel and @dhalperi)


docs/question_development/README.md line 11 at r2 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

I bet pybatfish generated names are sourced from it, too.

So fileParseStatus -> bf.q.fileParseStatus.

If you can confirm, consider updating again :p. Sorry.

Updated with my understanding.

Copy link
Copy Markdown
Member

@dhalperi dhalperi left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @arifogel)

@dhalperi dhalperi merged commit 660cd7c into master Sep 4, 2024
@dhalperi dhalperi deleted the ratulm-patch-1 branch September 4, 2024 05:54
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.

3 participants