fix: Create GitHub comment as table #3948
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Uffizzi Preview |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3948 +/- ##
=======================================
Coverage 95.98% 95.99%
=======================================
Files 1135 1135
Lines 36182 36220 +38
=======================================
+ Hits 34731 34771 +40
+ Misses 1451 1449 -2 ☔ View full report in Codecov by Sentry. |
api/integrations/github/constants.py
Outdated
| LINK_FEATURE_TEXT = "### This pull request is linked to a Flagsmith Feature (`%s`):\n" | ||
| LINK_FEATURE_TITLE = """**Flagsmith feature linked:** `%s` | ||
| Default Values:\n""" | ||
| FEATURE_TABLE_HEADER = """| Environment | Enabled | Value | Type | Updated (UTC) | |
There was a problem hiding this comment.
| FEATURE_TABLE_HEADER = """| Environment | Enabled | Value | Type | Updated (UTC) | | |
| FEATURE_TABLE_HEADER = """| Environment | Enabled | Value | Type | Last Updated (UTC) | |
This way, we won't have to prepend "Last Updated" to every timestamp in this column.
api/integrations/github/github.py
Outdated
| last_updated_string = LAST_UPDATED_FEATURE_TEXT % ( | ||
| datetime.datetime.now().strftime("%dth %b %Y %I:%M%p") | ||
| ) |
There was a problem hiding this comment.
I'm not a fan of the custom datetime format.
Let's consider either sticking to ISO8601 or using Django locale, i.e.:
from django.utils.formats import get_format
datetime_format = get_format('DATETIME_INPUT_FORMATS')[0]
|
Please cover the code reported as uncovered with tests, or add a |
I ended up adding the coverage. Let's keep this coverage while it is possible and is worth it. |
| expected_comment_body = ( | ||
| "**Flagsmith feature linked:** `feature_with_value`\n" | ||
| + "Default Values:\n" | ||
| + "| Environment | Enabled | Value | Last Updated (UTC) |\n" | ||
| + "| :--- | :----- | :------ | :------ |\n" | ||
| + f"| [Test Environment](https://example.com/project/{project.id}/" | ||
| + f"environment/{environment.api_key}/features?feature={feature_with_value.id}&tab=value) " | ||
| + "| ❌ Disabled | `value` | 2024-01-01 00:00:00 |\n" | ||
| ) |
There was a problem hiding this comment.
This is nice! nit: Python supports multiline strings so you could do this:
| expected_comment_body = ( | |
| "**Flagsmith feature linked:** `feature_with_value`\n" | |
| + "Default Values:\n" | |
| + "| Environment | Enabled | Value | Last Updated (UTC) |\n" | |
| + "| :--- | :----- | :------ | :------ |\n" | |
| + f"| [Test Environment](https://example.com/project/{project.id}/" | |
| + f"environment/{environment.api_key}/features?feature={feature_with_value.id}&tab=value) " | |
| + "| ❌ Disabled | `value` | 2024-01-01 00:00:00 |\n" | |
| ) | |
| expected_comment_body = ( | |
| "**Flagsmith feature linked:** `feature_with_value`\n" | |
| "Default Values:\n" | |
| "| Environment | Enabled | Value | Last Updated (UTC) |\n" | |
| "| :--- | :----- | :------ | :------ |\n" | |
| f"| [Test Environment](https://example.com/project/{project.id}/" | |
| f"environment/{environment.api_key}/features?feature={feature_with_value.id}&tab=value) " | |
| "| ❌ Disabled | `value` | 2024-01-01 00:00:00 |\n" | |
| ) |
Looks a bit nicer this way, IMO.
| if v.get("segment_name") is not None and v["segment_name"] != last_segment_name: | ||
| result += "\n" + LINK_SEGMENT_TITLE % (v["segment_name"]) | ||
| last_segment_name = v["segment_name"] | ||
| result += FEATURE_TABLE_HEADER |
There was a problem hiding this comment.
We should add a segment to the test or slap a # pragma: no cover on L108.
Thanks for submitting a PR! Please check the boxes below:
pre-committo check lintingdocs/if required so people know about the feature!Changes
How did you test this code?