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

[BUG] Dependencies don't specify python version (Issue 988) #1031

Closed
wants to merge 1 commit into from
Closed

[BUG] Dependencies don't specify python version (Issue 988) #1031

wants to merge 1 commit into from

Conversation

psaiz
Copy link

@psaiz psaiz commented Dec 15, 2021 •
edited by tmarkley
Loading

Description

Runs pathfix.py before creating the tar file, to fix the references that point to python instead of python3.
See #988

Please, note that this requires /usr/bin/pathfix.py. This comes with platform-python-devel.

Issues Resolved

#988

Check List

  • New functionality includes testing.
    • All tests pass
      • yarn test:jest
      • yarn test:jest_integration
      • yarn test:ftr
  • New functionality has been documented.
  • Commits are signed per the DCO using --signoff

@psaiz psaiz requested a review from a team as a code owner December 15, 2021 09:49
@kavilla kavilla changed the title Issue 988 [BUG] Dependencies don't specify python version (Issue 988) Dec 15, 2021
@kavilla kavilla linked an issue Dec 17, 2021 that may be closed by this pull request
[BUG] Dependencies don't specify python version #988
Closed
@kavilla
Copy link
Member

kavilla commented Dec 17, 2021 •
edited
Loading

Hello @psaiz would you be able to rebase and sign you commits based on Commits are signed per the DCO using --signoff

More information here: https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/CONTRIBUTING.md#developer-certificate-of-origin

@tmarkley
Copy link
Contributor

tmarkley commented Jan 5, 2022

Please, note that this requires /usr/bin/pathfix.py. This comes with platform-python-devel.

I'm not the biggest fan of adding yet another step to the build process, especially for something that is really just a bandaid. At the very least we need to add this requirement to documentation.

@psaiz
Copy link
Author

psaiz commented Jan 18, 2022

Hi again.

I asked the responsible of the dependencies to update the shebangs. The google-re2 has already done it, and that will allow node-re2 to do it. So, I do agree with Tommy that the extra step is not needed. Just putting the new versions might be enough. Let me look into that

@psaiz psaiz closed this Jan 18, 2022
@psaiz psaiz reopened this Jan 18, 2022
@psaiz
Copy link
Author

psaiz commented Jan 18, 2022

Ok, I left only the increase of version of node-gyp. Note that it still misses the update on node-re2. It might be better to wait until that one is ready, and do both in one go

AMoo-Miki pushed a commit to AMoo-Miki/OpenSearch-Dashboards that referenced this pull request Feb 10, 2022
@markov00
feat(xy_charts): render legend inside the chart (opensearch-project#1031
b271d09 
)

This commit adds the ability to display the legend inside the XY cartesian charts.
The <Settings> property legendPosition will take now an extended version of the position that allows increased customizability. The legend outside the chart only works with the following cases: `direction: vertical, vAlign: top, hAlign: left/right`, `direction: horizontal, hAlign:left, vAlign: top/bottom`,

fix opensearch-project#861
AMoo-Miki pushed a commit to AMoo-Miki/OpenSearch-Dashboards that referenced this pull request Feb 10, 2022
@semantic-release-bot
chore(release): 26.1.0 [skip ci]
fd910a5 
# [26.1.0](elastic/elastic-charts@v26.0.0...v26.1.0) (2021-03-26)

### Features

* **a11y:** add basic aria-label to canvas element ([opensearch-project#1084](elastic/elastic-charts#1084)) ([d4b3e4f](elastic/elastic-charts@d4b3e4f))
* **xy_charts:** render legend inside the chart ([opensearch-project#1031](elastic/elastic-charts#1031)) ([b271d09](elastic/elastic-charts@b271d09)), closes [opensearch-project#861](elastic/elastic-charts#861)
@tmarkley
Copy link
Contributor

Hi @psaiz, yeah I think we need to wait for a complete solution. Feel free to reopen this PR once that is ready.

@tmarkley tmarkley closed this Feb 21, 2022
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
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] Dependencies don't specify python version
3 participants
@psaiz @kavilla @tmarkley