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

LPS-134865 Upgrade tinycolor2 to 1.4.2 #1201

Closed
wants to merge 1 commit into from
Closed

LPS-134865 Upgrade tinycolor2 to 1.4.2 #1201

wants to merge 1 commit into from

Conversation

rolandpakai
Copy link

jQuery dependency coming from our clayui/color-picker which has a dependency of tinyColor2 (https://github.com/liferay/clay/blob/master/packages/clay-color-picker/package.json#L35), whose @1.4.1 version includes jQuery@1.9.1 (https://github.com/bgrins/TinyColor/blob/1.4.1/demo/jquery-1.9.1.js). Luckily for us, tinycolor2@1.4.2 removed any dependency from jQuery (bgrins/TinyColor@250a1e2)

More details: https://issues.liferay.com/browse/PTR-2475

cc @antonio-ortega

@liferay-continuous-integration
Copy link
Collaborator

CI is automatically triggering the following test suites:

  •     ci:test:relevant
  •     ci:test:sf

@liferay-continuous-integration
Copy link
Collaborator

✔️ ci:test:sf - 1 out of 1 jobs passed in 3 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 508408c3990d266b5d737cf0ce69b9d435fe5b3b

Sender Branch:

Branch Name: LPP-41667-master
Branch GIT ID: 1993f9e415eeb48d0f708e68371932a07f360357

1 out of 1jobs PASSED
1 Successful Jobs:
For more details click here.

@liferay-continuous-integration
Copy link
Collaborator

@matuzalemsteles
Copy link

Hey @rolandpakai, I wouldn't recommend making the change directly in yarn.lock, the best would be to make sure to update it in Clay and once done that would be reflected here. What do you think?

@bryceosterhaus
Copy link
Collaborator

bryceosterhaus commented Jun 28, 2021 •
edited
Loading

I agree with Matu here, we shouldn't be making any direct changes to our yarn.lock unless there is a corresponding dependency change in one of our package.json files. Updating tinycolor2 is best done in the Clay repo, and an even better option would be removing it entirely as a dependency from clay 😜 . And for what it is worth, it is my fault it's a dependency in Clay 😂

@matuzalemsteles
Copy link

Updating tinycolor2 is best done in the Clay repo, and an even better option would be removing it entirely as a dependency from clay 😜 . And for what it is worth, it is my fault it's a dependency in Clay 😂

haha 😂 that's true 😁, well, depending on whether we can analyze whether it's worth it, I didn't get to look at the tinycolor2 implementation, maybe it's worth it.

@liferay-continuous-integration
Copy link
Collaborator

✔️ ci:test:stable - 9 out of 9 jobs passed

✔️ ci:test:relevant - 19 out of 20 jobs passed in 2 hours 26 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 508408c3990d266b5d737cf0ce69b9d435fe5b3b

Upstream Comparison:

Branch GIT ID: 508408c3990d266b5d737cf0ce69b9d435fe5b3b
Jenkins Build URL: Acceptance Upstream DXP (master) #2051

ci:test:stable - 9 out of 9 jobs PASSED
9 Successful Jobs:
ci:test:relevant - 18 out of 20 jobs PASSED
18 Successful Jobs:
For more details click here.

This pull contains no unique failures.

Failures in common with acceptance upstream results at 508408c:
  1. test-portal-acceptance-pullrequest-batch(master)/unit-jdk8/0
    Job Results:

    2638 Tests Passed.
    1 Test Failed.

    1. AXIS_VARIABLE=0,label_exp=!master #230447
      1. com.liferay.portal.tools.ConfigurationEnvBuilderTest.testBuildContent
        junit.framework.AssertionFailedError: Run "ant generate-configuration-env" to regenerate modules/configuration-env.txt. expected:<...ompanyConfiguration_[enabled
#
LIFERAY_CONFIGURATION_OVERRIDE_COM_PERIOD_LIFERAY_PERIOD_LAYOUT_PERIOD_REPORTS_PERIOD_WEB_PERIOD_INTERNAL_PERIOD_CONFIGURATION_PERIOD__UPPERCASEL_AYOUT_UPPERCASER_EPORTS_UPPERCASEG_OOGLE_UPPERCASEP_AGE_UPPERCASES_PEED_UPPERCASEC_OMPANY_UPPERCASEC_ONFIGURATION_UNDERLINE_ENABLED

        

        com.liferay.layout.reports.web.internal.configuration.LayoutReportsGooglePageSpeedConfiguration_enabled
        

        

        LIFERAY_CONFIGURATION_OVERRIDE_COM_PERIOD_LIFERAY_PERIOD_LAYOUT_PERIOD_REPORTS_PERIOD_WEB_PERIOD_INTERNAL_PERIOD_CONFIGURATION_PERIOD__UPPERCASEL_AYOUT_UPPERCASER_EPORTS_UPPERCASEG_OOGLE_UPPERCASEP_AGE_UPPERCASES_PEED_UPPERCASEC_ONFIGURATION_UNDERLINE_ENABLED
        

        

        com.liferay.layout.reports.web.internal.configuration.LayoutReportsGooglePageSpeedGroupConfiguration_apiKey
        

        

        LIFERAY_CONFIGURATION_OVERRIDE_COM_PERIOD_LIFERAY_PERIOD_LAYOUT_PERIOD_REPORTS_PERIOD_WEB_PERIOD_INTERNAL_PERIOD_CONFIGURATION_PERIOD__UPPERCASEL_AYOUT_UPPERCASER_EPORTS_UPPERCASEG_OOGLE_UPPERCASEP_AGE_UPPERCASES_PEED_UPPERCASEG_ROUP_UPPERCASEC_ONFIGURATION_UNDERLINE_API_UPPERCASEK_EY
        

        

        com.liferay.layout.reports.web.internal.configuration.LayoutReportsGooglePageSpeedGroupConfiguration_enabled
        

        

        LIFERAY_CONFIGURATION_OVERRIDE_COM_PERIOD_LIFERAY_PERIOD_LAYOUT_PERIOD_REPORTS_PERIOD_WEB_PERIOD_INTERNAL_PERIOD_CONFIGURATION_PERIOD__UPPERCASEL_AYOUT_UPPERCASER_EPOR...
Test bundle downloads:

@liferay-continuous-integration
Copy link
Collaborator

@bryceosterhaus
Copy link
Collaborator

I'm gonna close to avoid an accidental forwards, but feel free to continue the conversation on this PR

@matuzalemsteles matuzalemsteles mentioned this pull request Jun 28, 2021
Closed
@bryceosterhaus
Copy link
Collaborator

Also, note that tinycolor2 was never using jquery for anything in its functionality, it was just there for their own demo site. So I don't think this would actually be an issue for that PTR.

@rolandpakai
Copy link
Author

Hi @matuzalemsteles

thank you for taking care for the upgrade in the clay.
Could you please let me known where can i check the new release of the @clayui/color-picker which contains the tinycolor2@1.4.2 ?

Thanks, Roland

@matuzalemsteles
Copy link

Hi @rolandpakai, I sent a new PR to update the package in Clay here, we're going to do a new release tomorrow, so you can follow the LPS-130580 ticket to follow up on the PR that we'll be sending out tomorrow.

@rolandpakai
Copy link
Author

Thank you @matuzalemsteles

@rolandpakai
Copy link
Author

Hi @matuzalemsteles

do you think i can backport @clayui/color-picker 3.31.0 to 73x without any issue or version dependency?

In 73x the current version is 3.5.0
https://github.com/liferay/liferay-portal-ee/blob/7.3.x/modules/apps/frontend-taglib/frontend-taglib-clay/package.json#L10

Thanks, Roland

@matuzalemsteles
Copy link

hey @rolandpakai, sorry for delay, looking at the changelog we haven't had many significant changes that might break something, so I think it's safe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
ci:test:relevant - success ci:test:sf - success ci:test:stable - success
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rolandpakai @liferay-continuous-integration @matuzalemsteles @bryceosterhaus