-
Notifications
You must be signed in to change notification settings - Fork 14
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.
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
Add developer overview #385
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #385 +/- ##
=======================================
Coverage 84.20% 84.20%
=======================================
Files 89 89
Lines 4040 4040
=======================================
Hits 3402 3402
Misses 638 638 ☔ View full report in Codecov by Sentry. |
Actually, as explained in the link you posted, it's not that Black's config isn't being used: Black still reads its config from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good birds-eye view - but it does comes dangerously close to being just a glossary of terms. Could Sphinx' glossary functionality be leveraged in some way here, perhaps?
https://sublime-and-sphinx-guide.readthedocs.io/en/latest/glossary.html
I'll play around a bit. Not sure if one can have more than one glossary, if ever we need another figure like this in the same docs. |
Hm, Sphinx |
or otherwise handled by the glue-service. | ||
|
||
The glue-service can either be built in to the source, or be completely | ||
standalone and pull changes from the source before it pushes them on. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"pushes them to Argus" instead of "pushes them on", maybe?
|
||
A notification profile is used by the server for sending notifications about | ||
a chosen subset of the incidents. It consists of one or more filters, one or | ||
more timeslots (for which periods of the day a profile is valid) and one or |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually just one timeslot - even after we make timeslots reusable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but suggestions from @johannaengland have merit and should be included.
4bff91b
to
6469b1a
Compare
Quality Gate passedIssues Measures |
6469b1a
to
00a5073
Compare
Turns out we had misunderstood how pre-commit excludes files. When running at least "black" from pre-commit, black doesn't use its own config files at all, including any list of excluded files.
Co-authored-by: Morten Brekkevold <100995+lunkwill42@users.noreply.github.com>
Co-authored-by: Morten Brekkevold <100995+lunkwill42@users.noreply.github.com>
00a5073
to
e24ca28
Compare
Quality Gate passedIssues Measures |
.. and fix problem with pre-commit! Excludes doesn't work as we thought.
At least for black, black's own config isn't used at all. It is pre-commit that selects which of the already indexed files to check and that is controlled with the "exclude", "language" and "types" arguments. "exclude" is a regular expression.
See https://stackoverflow.com/questions/61032281/exclude-some-files-on-running-black-using-pre-commit