Skip to content

Conversation

jvanasco
Copy link
Member

Adds an "append_delimiter" kwarg and checks the existing file to see if it is required.

There are likely better ways to handle this fix against #1679.

@CaselIT
Copy link
Member

CaselIT commented Jun 12, 2025

do we want to add a test?

@CaselIT CaselIT requested a review from sqla-tester June 12, 2025 07:32
Copy link
Collaborator

@sqla-tester sqla-tester left a comment

Choose a reason for hiding this comment

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

OK, this is sqla-tester setting up my work on behalf of CaselIT to try to get revision 8c0f417 of this pull request into gerrit so we can run tests and reviews and stuff

@sqla-tester
Copy link
Collaborator

New Gerrit review created for change 8c0f417: https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/5989

@jvanasco
Copy link
Member Author

do we want to add a test?

I couldn't find any test that covered an existing .toml file. I am not familiar enough with the test suite to write one. I certainly think a test would be beneficial though. test_command.py might be the right file for such a test.

@zzzeek
Copy link
Member

zzzeek commented Jun 12, 2025

i dont care about a test but it should have a changelog

@jvanasco
Copy link
Member Author

updated with changelog.

@CaselIT
Copy link
Member

CaselIT commented Jun 12, 2025

you placed the changelog in the changelog file, not in a separated fragment. I'm sure if it will cause issues when the changelog gets rendered. cc @zzzeek

@jvanasco
Copy link
Member Author

you placed the changelog in the changelog file, not in a separated fragment. I'm sure if it will cause issues when the changelog gets rendered. cc @zzzeek

Where should it go? The build here is different than sqlalchemy and I couldn't find instructions. Happy to make it conform.

@CaselIT
Copy link
Member

CaselIT commented Jun 12, 2025

like sqlalchemy, it should be a file in https://github.com/sqlalchemy/alembic/tree/main/docs/build/unreleased like #1672 (comment)

@jvanasco
Copy link
Member Author

Ah, sorry - I didn't see that directory.

SQLAlchemy has instructions here:

  • /changelog/README.txt

Alembic has them hidden, like this:

  • /changelog.rst
  • /unreleased/README.txt

Copy link
Collaborator

@sqla-tester sqla-tester left a comment

Choose a reason for hiding this comment

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

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision 20b2aca of this pull request into gerrit so we can run tests and reviews and stuff

@sqla-tester
Copy link
Collaborator

Patchset 20b2aca added to existing Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/5989

@sqla-tester
Copy link
Collaborator

Michael Bayer (zzzeek) wrote:

pep8-recheck

View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/5989

@sqla-tester
Copy link
Collaborator

Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/5989 has been merged. Congratulations! :)

artdogma added a commit to artdogma/pysettllembic that referenced this pull request Aug 27, 2025
Fixed rendering of ``pyproject.toml`` to include two newlines when
appending content to an existing file.  Pull request courtesy Jonathan
Vanasco.

Co-authored-by: Mike Bayer <[email protected]>
Fixes: #1679
Closes: #1680
Pull-request: sqlalchemy/alembic#1680
Pull-request-sha: 20b2aca0d2f6a8f5957063b950e45874343e9a99

Change-Id: Ic2e76a3f791dbda0bd61f09a2c7bafbcdab5eb7f
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