Repository navigation
[#132] Document Bugzilla migration - #204
Conversation
|
@sksingh2005 Can you take this one ? |
|
Sure. Will have a look |
sksingh2005
left a comment
There was a problem hiding this comment.
Thanks @Abdelrhman1326 docs-first looks good, and I verified the mapping against both billetsys' import pipeline and the Bugzilla 5.0/5.2 source. The field mapping, status mapping (lower(status) = 'closed' filter), duplicates, and requester fallback all match the code. Three corrections before merge:
- columnlist replaces, not appends. "Extra columns can be requested" reads as if &columnlist=opendate,bug_severity adds columns — it actually drops every default column, including short_desc. Please show a full columnlist in one example URL and warn explicitly.
- Version claims are wrong: these behaviors exist in 5.0, not 5.2+. Verified in both 5.0 and 5.2 sources:
- the leading space ( ---) is added by the CSV filter for any value starting with - (also = + @), not just resolution
- \ → \ (U+FF3C) is applied for logged-in users in 5.0 too
Suggest dropping the "5.2 and later" qualifiers.
3. Base URL optionality is missing, contradicting what we agreed in #132: filled → external_issue_link = base URL + show_bug.cgi?id= + bug_id; left empty → external_issue_link stays empty (traceable via source_key and the "Imported from Bugzilla bug #101" line). Please document that in the Import section.
Two optional nits: mention billetsys' 5 MB upload cap next to the 10000-bug truncation limitation (makes "split per product" doubly necessary), and state what the created_at timezone conversion targets — the pipeline only accepts ISO T timestamps, so the adapter must reformat Bugzilla's YYYY-MM-DD HH:MM:SS.
Good work, these are doc fixes only.
|
@Abdelrhman1326 Rebase is also needed |
|
Thanks for the review! @sksingh2005 I’ll rebase the branch and push the new commit very soon with all three corrections (columnlist replace behavior, dropping the “5.2+” qualifiers, base URL optionality) plus the two optional nits (5 MB cap, created_at format). |
|
@sksingh2005, @jesperpedersen I pushed the changes, and all three corrections and both nits are in:
I also merged |
|
@Abdelrhman1326 rebase |
9918626 to
2066018
Compare
|
@Abdelrhman1326 Remember to always rebase and squash. You can use orangu to help you. |
2066018 to
4138fcd
Compare
|
@sksingh2005 @jesperpedersen rebased onto main and squashed into one commit. Ready for another look. |
sksingh2005
left a comment
There was a problem hiding this comment.
Thanks @Abdelrhman1326! The updates look great and all review points are addressed.
Just one quick repo-convention item before merging: please also add your name to doc/manual/en/97-acknowledgement.md under Authors so it stays in sync with AUTHORS (and optionally drop the literal \ glyph in 60-import.md to keep XeLaTeX happy).
|
Thanks @sksingh2005 for the re-review! I've added my name and email to the Authors section in |
|
@Abdelrhman1326 Squash the commit into single one |
4dc2a9e to
529af95
Compare
|
Done @sksingh2005 . |
sksingh2005
left a comment
There was a problem hiding this comment.
LGTM! Approving the documentation and mapping specification. You can now go ahead with the code implementation in a follow-up PR.
|
Merged. |
|
Thanks @sksingh2005 for merging and for the warm welcome! Now that the proposal is in, I'll start working on the implementation. Midterms are next week, so I'll be balancing this implementation alongside my exam schedule and will post updates as I make progress on the PR, thanks again for the warm welcome! |
|
Sure. Take your time. Let us know when you stuck somewhere. Discussions are open as always. All the best. |
Documentation-first PR for #132, as discussed in the issue. It documents the Bugzilla CSV export, the proposed field and status mapping, and the known limitations. No code changes.
Refs #132