After 6 months of sitting unmerged, forward ports will now trigger a
monthly email.
Or would, if an email server was configured on the mergebot, but
adding this now should allow debugging its working and tracking the
generation of emails through the "Delivery Failed" collection. This
way if we ever decide to actually do the thing we can just enable an
SMTP server.
Fixes#801
Previously the alert would show the total number of outstanding
forward ports, which most people don't care about (if they're even
logged which probably isn't the case), so that was mostly useful
for... me.
Add the current user's tally to the message as well. If the user is
logged. Which is always, because the alert only shows up if the user
has read access to PRs.
Fixes#801
In the case where both author and reviewer of a PR have the same
team (which is very common e.g. self review or a team member approving
your PR) then the `outstanding_per_group` entry would be incremented
twice, leading to overcounting outstanding forward ports for the team.
Fixes#801
pycharm is a bit dumb so it considers `[ ]` to be unnecessary even in
`re.VERBOSE` regexes. But it has a bit of a point in that it's not
super clear, so inject the space characters by (unicode) name, the
behaviour should be the exact same but the regexes should be slightly
less opaque.
- only remind weekly initially (not daily)
- root reminders on the forward port's creation, not the source's
merge date
- cap reminder interval at 4 weeks (instead of doubling every time)
- track reminders on forward ports, don't share between siblings
- remove `forwardport_updated_before` from the testing system, it's
now possible to just update `reminder_next` to a past date and see
if it gets triggered (it should to nothing on sources, or on forward
port in a state which does not warrant concern)
Fixes#801
Link stagings to the staging commits instead of the commits directly
in order to have the repository name and not need to try the commit on
every repository.
Also make the trees editable, in combination with `readonly="1"` this
does not make the trees editable but does prevent clicking on a row to
open a dialog with the linked objects, which is convenient if one
wants to copy the commit hash.
Having the staged and merged heads via an easy to reach endpoint is
nice, but having *just* the hashes is not as it requires basically
exhaustively checking commits against repos to see which is
where. That's annoying.
Make the head serializations a map of repository to commit
hash. That's a lot more convenient.
Hopefully this is the last fix to the patcher. From the start of the
implementation I relied on the idea that `git show` was adding a line
composed of a single space (and a newline) before and after the patch
message, as that is what I observed in my terminal, and it's
consistent with RFC 3676 signatures (two dashes, a space, and a
newline).
Turns out that single space, while present in my terminal indeed, was
completely made up by `less(1)`. `git show` itself doesn't generate
that, neither does it appear when using most pagers, or even when
piping the output of `less` into something (a file, an other pager,
...). It's pretty much just something `less(1)` sends to a terminal
during interactive sessions to fuck with you.
Fixes#1037
- use lingo `staged` and `merged` for clarity (instead of `heads` and
`commits`)
- convert from json-rpc to regular HTTP to make easier to call
- add the staged and merged heads when returning staging data
- return more complete objects (rather than a list of ids) when
matching stagings based on commits / heads
Fixes#1018
The old controller system required `type='json'` which only did
JSON-RPC and prevented returning proper responses.
As of 17.0 this is not the case anymore, `type='http'` controllers can
get `content-type: application/json` requests just fine and return
whatever they want. So change that:
- `type='json'`.
- Return `Response` objects with nice status codes (and text
mimetypes, as otherwise werkzeug defaults to html).
- Update ping to bypass normal flow as otherwise it requires
authentication and event sources which is annoying (it might be a
good idea to have both in order to check for configuration, however
it's not possible to just send a ping via the webhook UI on github
so not sure how useful that would be).
- Add some typing and improve response messages while at it.
Note: some "internal" errors (e.g. ignoring event actions) are
reported as successes because as far as I can tell webhooks only
support success (2xy) / failure (4xy, 5xy) and an event that's ignored
is not really *failed* per se.
Some things are reported as failures even though they are on the edge
because that can be useful to see what's happening e.g. comment too
large or unable to lock rows.
Fixes#1019
Probably should create a mixin for this: when a model is used as a
task queue for a cron, the cron should automatically be triggered on
creation. Requiring an explicit trigger after a creation is error
prone and increase the risks that some of the triggers will be
forgotten/missed.
Most of the repos we create we want to hook into the odoo instance,
but in some rare cases we want repositories for just push/pull, in
that case webhooks configuration just creates log noise (as the events
get rejected on intake).
- add support for reading commit entries
- add support for reading tree entries in non-recursive mode
Previously the helper would only return blob entries, which does not
allow testing submodule support.
Seems like remnants of old debugging of the rate limiting, and can
create odd situations when it fires as it replaces `sys.stdout` with
an in-memory buffer, which doesn't have a `fileno()`, which breaks
`server._move`.
Adds a very limited ability to try and look for false positive /
non-determinstic staging errors. It tries to err on the side of
limiting false false positives, so it's likely to miss many.
Currently has no automation / reporting, just sets a flag on the
stagings which are strongly believed to have failed due to false
positives.
While at it, add link between a "root" staging and its splits. It's
necessary to clear the "false positive" flag, and surfacing it in the
UI could be useful one day.
Fixes#660
Rather than add individual tunnel methods to conftest, just allows
specifying a tunnel script and have that do whatever.
Protocol is uncomplicated: workers run the `$tunnel` with an arbitrary
port, script should create a tunnel to `localhost:$port`, print the
ingress of the tunnel to `STDOUT` with a terminating newline then
close `STDOUT`, and wait for `SIGINT` or `SIGTERM`, doing whatever
cleanup they need when receiving either of those.
`ngrok` and `localtunnel` adapter scripts are provided out of the box,
though the ngrok one doesn't *really* work when using xdist without a
pre-spawned ngrok worker. Then again using xdist against github actual
is suicidal (because concurrency limits + rate limits) so likely
irrelevant.
Fixes#729
Requires parsing the commit messages as github plain doesn't collate
the info from there, but also the descriptions: apparently github only
adds those to the references if the PR targets the default
branch. That's not a limitation we want.
Meaning at the end of the day, the only thing we're calling graphql
for is explicit manual linking, which can't be tested without
extending the github API (and then would only be testable on DC), and
which I'm not sure anyone bothers with...
Limitations
-----------
- Links are retrieved on staging (so if one is added later it won't be
taken in account).
- Only repository-local links are handled, not cross-repository.
Fixes#777
In that case, ignore the reopen, close the PR, and tell the idiot to
fuck off.
Also the case where a PR is reopened while its batch is staged was
already handled, but probably not tested: it was implicit in
forcefully updating the HEAD of the PR, which triggers an unstage
since c8a06601a7.
Now that scenario is tested, which should lower the odds of breaking
it in the future.
Fixes#965
Identifying build errors by fingerprint is not enough.
Most of the times we want to link build errors based on some criteria.
With this commit, a build error can be qualified by using regular
expressions that will extract informations.
Those informations will be stored in a jsondict field. That way, we can
find simmilar build errors when they share some qualifiers.
To extract information, the regular expression must use named group
patterns.
e.g:
`^Tour (?P<tour_name>\w+) failed at step (?P<tour_step>.+)`
The above regular expression should extract the tour name and tour step
from a build error and store them like:
`{ "tour_name": "article_portal_tour", "tour_step": "clik on 'Help'" }`
The field can be queried to find similar errors. Also, a button is added
on the Build Error Form to search for errors that share the same
qualifiers.
The `to_pr` helper was added a *while* ago to replace the pretty
verbose process of looking a PR with the right number in the right
repository after a `make_pr`. However while I did some ad-hoc
replacement of existing `search` calls as I had to work with existing
tests I never did a full search and replace to try and excise searches
from the test suite.
This is now done. I've undoubtedly missed a few, but a hundred-odd
lines of codes have been simplified.
BS5 namespaced the data-attributes it uses to trigger JS
behaviours. So for dropdowns `@data-toggle` doesn't do anything
anymore, one has to use `@data-bs-toggle`. Missed that while testing
the migrations.
Also seems like `@aria-expanded` was misapplied when I added the
dropdowns:
> When a menu is displayed, the button object that toggles the
> visibility of that menu has aria-expanded="true" set. When the menu
> is hidden, aria-expanded can be omitted. If specified when the menu
> is hidden, it should be set as aria-expanded="false".
Since the dropdowns are hidden by default, the button should be
`@aria-expanded="false"`.
As the number of projects is starting to grow pretty large, provide
quick links and stable jump targets for projects on the home page /
main dashboard.
Fixes#991
- don't *fail* in `_compute_identity`, it causes issues when the token
is valid but doesn't have `user:email` access as the request is
aborted and saving doesn't work
- make `github_name` and `github_email` required rather than ad-hoc
requiring them in `_compute_identity` (which doesn't work correctly)
- force copy of `github_name` and `github_email`, with o2ms being
!copy this means duplicating projects now works out of the box (or
should...)
Currently errors in `_compute_identity` are reported via logging which
is not great as it's not UI visible, should probably get moved to
chatter eventually but that's not currently enabled on projects.
Fixes#990
Show patch metadata on the patch screen, so it's easier to understand
what the parser sees in case of issues.
Behaviour is not *entirely* consisten, `file_ids` is correctly set but
it looks like during the initial `web_read` it gets stripped out in at
least some cases and the files list is empty even though files have
been found in the patch. nm.
Fixes#987
- Apparently if a user is on windows the ACE editor can swap out their
line end from unix to windows. The patch parsers were predicated
upon all patches being in unix mode (because git, and diff).
Fixup both parsers to convert windows-style line end to unix before
trying to parse the patch data. Also add a few fallbacks to limit
the odds of an unhelpful `StopIteration` (though that might hide
errors more than reveal them...)
- Make sure we support `format-patch --no-signature`, just requires
using the correct partition direction: I assume I used `rpartition`
as a form of micro-optimisation *but*
- If the separator is not found the "patch body" ends up in the
third parameter rather than the first, which makes the fallback
difficult.
- There doesn't seem to be anything preventing *multiple* signature
separators in a message, and logically the first one should hold
and the rest is all part of the signature.
As a result, for both reasons we need to look *forwards* for the
signature separator, not backwards. Hence `str.partition`.
Fixes#992
Turns out to not work well in 17.0, and after consideration moc hasn't
really used the auto-update feature (or the required-prs gate in
general to be honest), usually he knows what PRs he's waiting for and
only validates once he's confirmed every which way.
So it's probably not worth fixing the thing. According to jpp, this
should probably use something based on bus subscriptions to update
just the field (though tbf the `root.update` call doesn't really seem
to be "deep" anymore, so in reality rather than update the *form*'s
record I should probably have tried reloading the required_pr_ids
records to fetch the new color).
Closes#997
The template part was not reached only in certain cases and regardless
the empty collapse toggle has not been visible for a while..
As handling subbuild error logs adds unnecessary load when rendering the
page anyways (most people will not display the subbuild errors
regardless if the collapse is showing or not), we decided to remove it
completely.
Until now the favicon was handled by the `/favicon.ico` route made
available by `website`.
This commit adds the different favicons and logic to be able to display
the state of the current page through it.
Duplicate error content should not happens ... but it does.
With this commit, a server actions allows to relink error contents and
thus removes error contents having the same fingerprint.