Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Custom change-logger #89

Open
wants to merge 30 commits into
base: main
Choose a base branch
from
Open

Custom change-logger #89

wants to merge 30 commits into from

Conversation

Shivansh-007
Copy link
Member

@Shivansh-007 Shivansh-007 commented Oct 17, 2021 •
edited

Closes #87

To Do (Now):

  • Move constants to config.toml
  • Add GitHub workflow for checking news
  • Document the code
  • Clean and remove redundant code

To Do (Will be done once the current API/design is finalised and reviewed):

  • Add documentation
  • Port existing change-log

@Shivansh-007 Shivansh-007 added p: normal Normal Priority s: WIP Waiting for author to address a review or respond to a comment t: feature Relating to the functionality of the application. a: tools Development related to our toolchain (Docker, poetry, flake8) hacktoberfest-accepted labels Oct 17, 2021
@Shivansh-007 Shivansh-007 marked this pull request as ready for review October 18, 2021 10:13
@Shivansh-007 Shivansh-007 added s: needs review Ready for review and merge and removed s: WIP Waiting for author to address a review or respond to a comment labels Oct 18, 2021
@Shivansh-007 Shivansh-007 force-pushed the feat/custom-changelogger branch 7 times, most recently from eda68f0 to d8b5e69 Compare October 19, 2021 11:00
@onerandomusername onerandomusername mentioned this pull request Nov 8, 2021
Merged
onerandomusername
onerandomusername requested changes Nov 17, 2021
View reviewed changes
scripts/news/__main__.py Outdated
)


PR_ENDPOINT = "https://api.github.com/repos/discord-modmail/modmail/pulls/{number}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this org and repo be part of the configuration?

scripts/news/__main__.py Outdated Show resolved Hide resolved
scripts/news/__main__.py Outdated Show resolved Hide resolved
scripts/news/__main__.py Outdated Show resolved Hide resolved
scripts/news/__main__.py Outdated Show resolved Hide resolved
scripts/news/utils.py Outdated Show resolved Hide resolved
scripts/news/utils.py Outdated Show resolved Hide resolved
scripts/news/utils.py Outdated Show resolved Hide resolved
scripts/news/utils.py Show resolved Hide resolved
scripts/news/utils.py Outdated Show resolved Hide resolved
Shivansh-007 and others added 2 commits November 27, 2021 12:04
@Shivansh-007
Merge branch 'main' into feat/custom-changelogger
803d3c0
@onerandomusername
deps: run poetry lock --no-update
aa11a32 
Signed-off-by: onerandomusername <genericusername414+git@gmail.com>
onerandomusername
onerandomusername requested changes Nov 27, 2021
View reviewed changes
Copy link
Member

@onerandomusername onerandomusername left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two other notes:
1: add a news (or two, since there's the workflow as well) entry point to scripts.main
2: Add a news entry point to the taskipy config

requirements.txt Show resolved Hide resolved
scripts/news/__main__.py Show resolved Hide resolved
scripts/news/__main__.py Show resolved Hide resolved
scripts/news/check_news_workflow/__main__.py
return status


if __name__ == "__main__":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

debugging, and trying to figure out what happens, in addition to being a simple check before commiting, with the ability to pass a pr number, or something.

scripts/news/utils.py Outdated Show resolved Hide resolved
scripts/news/utils.py Show resolved Hide resolved
scripts/news/utils.py Show resolved Hide resolved
tox.ini Outdated Show resolved Hide resolved
@coveralls
Copy link
Collaborator

coveralls commented Dec 8, 2021 •
edited

Coverage Status

Coverage remained the same at 78.756% when pulling 1d149db on feat/custom-changelogger into 7347f59 on main.

@discord-modmail discord-modmail deleted a comment from codecov bot Dec 8, 2021
onerandomusername
onerandomusername requested changes Dec 14, 2021
View reviewed changes
Copy link
Member

@onerandomusername onerandomusername left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making a fragment was nice and fluent.

Building the actual change notes needs some work.
See #87 description under naming scheme and compiler script for the spec

@discord-modmail discord-modmail deleted a comment from codecov bot Dec 14, 2021
@codecov
Copy link

codecov bot commented Dec 14, 2021

Codecov Report

Merging #89 (b164477) into main (4b07917) will decrease coverage by 0.28%.
The diff coverage is n/a.

❗ Current head b164477 differs from pull request most recent head 1d149db. Consider uploading reports for the commit 1d149db to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main      #89      +/-   ##
==========================================
- Coverage   70.90%   70.62%   -0.29%     
==========================================
  Files          33       32       -1     
  Lines        1619     1593      -26     
  Branches      199      190       -9     
==========================================
- Hits         1148     1125      -23     
+ Misses        439      437       -2     
+ Partials       32       31       -1
Impacted Files Coverage Δ
tests/modmail/test_logs.py 100.00% <0.00%> (ø)
tests/modmail/utils/test_embeds.py 100.00% <0.00%> (ø)
tests/modmail/conftest.py

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7347f59...1d149db. Read the comment docs.

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

@bast0006 bast0006 Awaiting requested review from bast0006

@onerandomusername onerandomusername Awaiting requested review from onerandomusername onerandomusername is a code owner

Requested changes must be addressed to merge this pull request.

Assignees

@Shivansh-007 Shivansh-007

Labels
a: tools Development related to our toolchain (Docker, poetry, flake8) hacktoberfest-accepted p: normal Normal Priority s: needs review Ready for review and merge t: feature Relating to the functionality of the application.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Changelog Scripts
3 participants
@Shivansh-007 @onerandomusername @coveralls