Skip to content

Add developer overview#385

Open
hmpf wants to merge 4 commits into
Uninett:masterfrom
hmpf:developer-overview
Open

Add developer overview#385
hmpf wants to merge 4 commits into
Uninett:masterfrom
hmpf:developer-overview

Conversation

@hmpf

@hmpf hmpf commented Mar 29, 2022

Copy link
Copy Markdown
Contributor

.. 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

@codecov-commenter

codecov-commenter commented Mar 29, 2022

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.20%. Comparing base (d114b38) to head (e24ca28).
⚠️ Report is 839 commits behind head on master.

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 Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@lunkwill42

Copy link
Copy Markdown
Member

.. 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

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 pyproject.toml in our case. However, Blacks exclude-option is only used to exclude files when Black is discovering the file tree itself. If a filename is explicitly put as an argument to the black command line (which seems to be what the pre-commit framework is doing), Black's exclude option doesn't apply at all.

@lunkwill42 lunkwill42 left a comment

Copy link
Copy Markdown
Member

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

Comment thread docs/development/birdseye-view.rst Outdated
Comment thread docs/development/birdseye-view.rst Outdated
@hmpf

hmpf commented Apr 25, 2022

Copy link
Copy Markdown
Contributor Author

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?

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.

@hmpf

hmpf commented Apr 25, 2022

Copy link
Copy Markdown
Contributor Author

Hm, Sphinx glossary is not hierarchical, so there would be no good way to both discuss sources and types of sources without a major rewrite.

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.

Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Contributor

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

@hmpf
hmpf force-pushed the developer-overview branch from f817669 to 4bff91b Compare May 3, 2022 11:31

@lunkwill42 lunkwill42 left a comment

Copy link
Copy Markdown
Member

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.

@hmpf hmpf added the rc-future label Oct 10, 2022
@lunkwill42
lunkwill42 removed the request for review from stveit October 14, 2022 06:23
@hmpf hmpf added the paused Assignee is busy with things of higher priority label Nov 15, 2022
@hmpf hmpf self-assigned this Nov 16, 2022
@hmpf
hmpf force-pushed the developer-overview branch from 4bff91b to 6469b1a Compare April 25, 2024 13:06
@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@github-actions

Copy link
Copy Markdown

Test results

       5 files         1 errors  328 suites   11m 41s ⏱️
   462 tests    461 ✔️ 1 💤 0
1 848 runs  1 844 ✔️ 4 💤 0

For more details on these parsing errors, see this check.

Results for commit 6469b1a.

@hmpf
hmpf force-pushed the developer-overview branch from 6469b1a to 00a5073 Compare September 13, 2024 10:49
hmpf and others added 4 commits October 4, 2024 10:07
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>
@hmpf
hmpf force-pushed the developer-overview branch from 00a5073 to e24ca28 Compare October 4, 2024 08:09
@sonarqubecloud

sonarqubecloud Bot commented Oct 4, 2024

Copy link
Copy Markdown

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

Labels

paused Assignee is busy with things of higher priority rc-future

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants