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

CORE-3326 Numero sign is a symbol in Russian #1320

Merged
merged 2 commits into from Jan 24, 2022
Merged

Conversation

tolix
Copy link
Contributor

@tolix tolix commented Aug 14, 2020

Numero sign in Russian it's a regular symbol (https://en.wikipedia.org/wiki/Numero_sign)). Problem was described here https://liquibase.jira.com/browse/CORE-3326 and actual for lb 4.0 version


name: Pull Request
about: Create a report to help us improve
title: 'CORE-3326 Numero sign is a symbol in Russian'
labels: Status:Discovery
assignees: ''


Environment

Liquibase Version: 4.0 and earlier

Liquibase Integration & Version: <Pick one: CLI, maven, gradle, spring boot, servlet, etc.>

Liquibase Extension(s) & Version: any

Database Vendor & Version: any

Operating System Type & Version: any

Pull Request Type

  • Bug fix (non-breaking change which fixes an issue.)
  • Enhancement/New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

https://liquibase.jira.com/browse/CORE-3326

Fast Track PR Acceptance Checklist:

<

!--- If you're unsure about any of these, just ask us in a comment. We're here to help|width=200,height=183!

-->


Dev Handoff Notes (Internal Use)

Links

Testing

Dev Verification

Code review and ensured existing automated tests pass. Did not add extra unit test since we don't test every character

Test Requirements (Liquibase Internal QA)

Manual Tests

Verify update-sql works correctly when Numero sign is in comment.

  • liquibase update-sql --changelog-file lb562-changelog.xml --labels comment
  • no error is thrown
  • generated sql correctly displays Numero sign

Verify update is successful when Numero sign is in comment.

  • liquibase update --changelog-file lb562-changelog.xml --labels comment
  • no error is thrown
  • comment in DATABASECHANGELOG table correctly displays Numero sign

Verify update-sql works correctly when Numero sign is in insert tag.

  • liquibase update-sql --changelog-file lb562-changelog.xml --labels insert
  • no error is thrown
  • generated sql correctly displays Numero sign

Verify update is successful when Numero sign is in insert tag.

  • liquibase update --changelog-file lb562-changelog.xml --labels insert
  • no error is thrown
  • inserted value correctly displays Numero sign

Verify update-sql works correctly when Numero sign is in sql tag.

  • liquibase update-sql --changelog-file lb562-changelog.xml --labels sql
  • no error is thrown
  • generated sql correctly displays Numero sign

Verify update is successful when Numero sign is in sql tag.

  • liquibase update --changelog-file lb562-changelog.xml --labels sql
  • no error is thrown
  • inserted value correctly displays Numero sign

Verify update-sql works correctly when Numero sign is in sqlFile tag.

  • liquibase update-sql --changelog-file lb562-changelog.xml --labels sqlfile
  • no error is thrown
  • generated sql correctly displays Numero sign

Verify update is successful when Numero sign is in sqlFile tag.

  • liquibase update --changelog-file lb562-changelog.xml --labels sqlfile
  • no error is thrown
  • SquareBracketsTest procedure successfully created

Verify update-sql works correctly when Numero sign is in formatted sql changelog

  • liquibase update-sql --changelog-file lb562-changelog.sql
  • no error is thrown
  • generated sql correctly displays Numero sign

Verify update is successful when Numero sign is in formatted sql changelog

  • liquibase update --changelog-file lb562-changelog.sql
  • no error is thrown
  • AnotherTest procedure successfully created

Verify generate-changelog correctly handles Numero sign

  • liquibase generate-changelog --changelog-file lb562-generated.xml --diff-types tables,columns,data
  • no error is thrown
  • execute drop-all command
  • run update with generated changelog
    • liquibase update --changelog-file lb562-generated.xml
  • update is succsessful
  • table TEST_TABLE1 is created and filled with data that correctly displays Numero sign

Automated Tests

No new functional tests required for this fix.

┆Issue is synchronized with this Jira Bug by Unito

Numero sign in Russian it's a regular symbol (https://en.wikipedia.org/wiki/Numero_sign). Problem was described here https://liquibase.jira.com/browse/CORE-3326 and actual for lb 4.0 version
@molivasdat
Copy link
Contributor

Thanks @tolix
Thanks for your pull request!

Here’s what happens next:

A member of the Liquibase team will take a look at your contribution and may suggest:

The PR will be prioritized according to our internal development and testing capacity.

We’ll let you know when it’s ready to move to the next step or if any changes are needed.

@molivasdat molivasdat added DBAll ImpactMedium IntegrationAny RiskLow Trivial changes in spelling, documentation changes, focused bug fixes, etc. TypeBug Severity3 labels Aug 17, 2020
@netlink128
Copy link

Hello! When we can get this fix?
Thank you!

@netlink128
Copy link

Hello! When we can get this fix?
This is huge stop factor for us today.
Do you have any forecasts?

@molivasdat molivasdat added this to To Do in Conditioning++ via automation Dec 28, 2021
@molivasdat
Copy link
Contributor

I have add this to the list of issues to process.
Needs to merge with existing currency changes that were added to this file.

@liquibase liquibase deleted a comment from codecov bot Jan 5, 2022
…ix-patch-1

# Conflicts:
#	liquibase-core/src/main/javacc/liquibase/util/grammar/SimpleSqlGrammar.jj
Copy link
Contributor

@nvoxland nvoxland left a comment

Choose a reason for hiding this comment

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

Changes look good. I resolved the conflict and pushed it to the fork

@nvoxland nvoxland changed the base branch from master to 1_9 January 5, 2022 18:32
@nvoxland nvoxland changed the base branch from 1_9 to master January 5, 2022 18:33
@nvoxland nvoxland moved this from To Do to Code Review in Conditioning++ Jan 5, 2022
@suryaaki2 suryaaki2 moved this from Code Review to Ready for Handoff (In JIRA) in Conditioning++ Jan 6, 2022
@sync-by-unito sync-by-unito bot assigned nvoxland and yodzhubeiskyi and unassigned nvoxland Jan 10, 2022
@nvoxland nvoxland merged commit b8b6047 into liquibase:master Jan 24, 2022
Conditioning++ automation moved this from Ready for Handoff (In JIRA) to Done Jan 24, 2022
@netlink128
Copy link

Hello!
When we can get it in release?

@nvoxland nvoxland added this to the v4.8.0 milestone Feb 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DBAll ImpactMedium IntegrationAny IntegrationCLI RiskLow Trivial changes in spelling, documentation changes, focused bug fixes, etc. Severity3 SyncTicket TypeBug
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

8 participants