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.

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

Support python 3.13 #729

Merged
merged 15 commits into from
Oct 16, 2024
Merged

Conversation

Firdous2307
Copy link
Contributor

@Firdous2307 Firdous2307 commented Oct 13, 2024

Add support for Python 3.13

This PR adds support for Python 3.13 to the icalendar project. It includes updates to the test configurations, project metadata, and documentation.

Changes made:

  1. Added Python 3.13 to the GitHub Actions test matrix
  2. Included py313 environment in tox configuration
  3. Updated classifiers in pyproject.toml to include Python 3.13
  4. Updated README with Python 3.13 compatibility information
  5. Added documentation on the process of updating Python versions

Documentation:

  • Updated README with new Python version support
  • Added section in docs/maintenance.rst about updating Python versions

This PR addresses issue #723.


📚 Documentation preview 📚: https://icalendar--729.org.readthedocs.build/

@niccokunzmann
Copy link
Member

@Firdous2307 Thanks! I wonder why there are no merge conflicts.. I will update the branch.

Could you fix the changelog? The last change was in the already released version.
Thanks for the documentation!

@coveralls
Copy link

coveralls commented Oct 13, 2024

Pull Request Test Coverage Report for Build 11364486897

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.01%) to 97.349%

Totals Coverage Status
Change from base Build 11317448813: 0.01%
Covered Lines: 3520
Relevant Lines: 3612

💛 - Coveralls

@Firdous2307
Copy link
Contributor Author

@niccokunzmann no problem , I have fixed the changelog

Copy link
Member

@niccokunzmann niccokunzmann left a comment

Choose a reason for hiding this comment

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

Thanks! I added a few comments to clear possible confusion about what I meant. 6.0.1 is already released. We cannot change the changelog for that except take that one line about 3.13 out that was accidentally not in 6.0.2 but added to 6.0.1.

CHANGES.rst Outdated

Breaking changes:

- ...

New features:

- ...

Bug fixes:
Copy link
Member

Choose a reason for hiding this comment

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

These lines need to stay... That is where people fill things in until we release.

CHANGES.rst Outdated
------------------

New features:

- Added ``Event.end``, ``Event.start``, ``Event.dtstart``, and ``Event.dtend`` attributes. See `Issue 662 <https://github.com/collective/icalendar/issues/662>`_.
- Test compatibility with Python 3.13
Copy link
Member

Choose a reason for hiding this comment

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

that would need removing

docs/maintenance.rst Show resolved Hide resolved
CHANGES.rst Outdated Show resolved Hide resolved
CHANGES.rst Show resolved Hide resolved
@niccokunzmann
Copy link
Member

Thanks for working on it! It is nice for me to collaborate instead of doing it myself! I hope I learn to be better at communicating this way, too :)

@niccokunzmann
Copy link
Member

Please let me know when you would like me to review/merge!

@Firdous2307
Copy link
Contributor Author

Thanks for working on it! It is nice for me to collaborate instead of doing it myself! I hope I learn to be better at communicating this way, too :)

Thank you! Glad you're enjoying the collaboration. You're doing great, and it only gets better with practice! 😊

@Firdous2307
Copy link
Contributor Author

@niccokunzmann It's ready for review!!

1 similar comment
@Firdous2307
Copy link
Contributor Author

@niccokunzmann It's ready for review!!

CHANGES.rst Outdated Show resolved Hide resolved
@niccokunzmann
Copy link
Member

niccokunzmann commented Oct 13, 2024

Please have a look at the changelog file
https://github.com/collective/icalendar/blob/30197304a45053dc4cb31e279b992cb45c46d5e1/CHANGES.rst
You can view this file and you will see inconsistencies at the start.

your changes should be added to the top heading:

https://github.com/collective/icalendar/blob/main/CHANGES.rst

Everything else would be left untouched - except for the change for 3.13 that landed in the wrong release.

Co-authored-by: Nicco Kunzmann <[email protected]>
@niccokunzmann
Copy link
Member

Hm... No problem if you want to not spend so much time just on the changelog... I create a PR for you ....

@niccokunzmann
Copy link
Member

I could not create a PR but I could commit directly.

I think, #730 (comment) is valid for this PR, though, could you use the proper RST way and then, I think, it is fine to merge!

@stevepiercy
Copy link
Member

The changes in this PR do not do what the description says.

@niccokunzmann
Copy link
Member

@stevepiercy I think that is ok. @Firdous2307 is contributing for the first time and it does not need to be too perfect. This is a follow-up PR from the on made already to close #723. In that sense, it is not too bad. We had the changes in here but they were removed because of duplication.

@Firdous2307
Copy link
Contributor Author

@niccokunzmann Any more changes needed to make?

Copy link
Member

@niccokunzmann niccokunzmann left a comment

Choose a reason for hiding this comment

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

I will apply these changes and merge it. Thanks!

CHANGES.rst Outdated Show resolved Hide resolved
docs/maintenance.rst Outdated Show resolved Hide resolved
docs/maintenance.rst Outdated Show resolved Hide resolved
docs/maintenance.rst Outdated Show resolved Hide resolved
docs/maintenance.rst Outdated Show resolved Hide resolved
docs/maintenance.rst Outdated Show resolved Hide resolved
@niccokunzmann niccokunzmann merged commit 903f3dd into collective:main Oct 16, 2024
18 checks passed
@Firdous2307
Copy link
Contributor Author

I will apply these changes and merge it. Thanks!

@niccokunzmann You are welcome, had a fun experience working on this issue

@niccokunzmann
Copy link
Member

@Firdous2307 Thanks for your work!

I squashed the commits into one and so you will need to start from collective/icalendar's main branch for new PRs.

@niccokunzmann
Copy link
Member

If you are doing the hacvktoberfest, have a look if this shows up as a contribution - it should. If it does not , let me know :)

@Firdous2307
Copy link
Contributor Author

If you are doing the hacvktoberfest, have a look if this shows up as a contribution - it should. If it does not , let me know :)

@niccokunzmann Yes I am, ideally it shows up as a contribution but they review it in 7 days. I will keep you posted on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants