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

Bump version and add notebooks context menu fix for 1.0.1.0 patch #142

Merged
merged 3 commits into from
Aug 25, 2021
Merged

Bump version and add notebooks context menu fix for 1.0.1.0 patch #142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
765f2e9
bump version and add notebooks context menu fix for 1.0.1.0 patch
joshuali925 Jul 6, 2021
9999f0a
update core to use 1.0 tag
davidcui1225 Aug 25, 2021
020554d
change opensearch version to 1.0.0 to auto-pick tag
davidcui1225 Aug 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/reports-scheduler-test-and-build-workflow.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name: Test and Build Reports Scheduler
on: [push, pull_request]

env:
OPENSEARCH_VERSION: '1.0'
OPENSEARCH_VERSION: '1.0.0'
COMMON_UTILS_VERSION: '1.0'
JOB_SCHEDULER_VERSION: '1.0'

Expand Down Expand Up @@ -37,7 +37,7 @@ jobs:
path: common-utils
- name: Build common-utils
working-directory: ./common-utils
run: ./gradlew publishToMavenLocal -Dopensearch.version=${{ env.OPENSEARCH_VERSION }}.0
run: ./gradlew publishToMavenLocal -Dopensearch.version=${{ env.OPENSEARCH_VERSION }}

# dependencies: job-scheduler
- name: Checkout job-scheduler
Expand All @@ -48,7 +48,7 @@ jobs:
path: job-scheduler
- name: Build job-scheduler
working-directory: ./job-scheduler
run: ./gradlew publishToMavenLocal -Dopensearch.version=${{ env.OPENSEARCH_VERSION }}.0 -Dbuild.snapshot=false
run: ./gradlew publishToMavenLocal -Dopensearch.version=${{ env.OPENSEARCH_VERSION }} -Dbuild.snapshot=false

# reports-scheduler
- name: Checkout Reports Scheduler
Expand All @@ -57,7 +57,7 @@ jobs:
- name: Build with Gradle
run: |
cd reports-scheduler
./gradlew build -Dopensearch.version=${{ env.OPENSEARCH_VERSION }}.0
./gradlew build -Dopensearch.version=${{ env.OPENSEARCH_VERSION }}

- name: Upload coverage
uses: codecov/codecov-action@v1
Expand Down
2 changes: 1 addition & 1 deletion dashboards-reports/opensearch_dashboards.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"id": "reportsDashboards",
"version": "1.0.0.0",
"version": "1.0.1.0",
"opensearchDashboardsVersion": "1.0.0",
"requiredPlugins": ["navigation", "data", "opensearchDashboardsUtils"],
"optionalPlugins": ["share"],
Expand Down
2 changes: 1 addition & 1 deletion dashboards-reports/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "reports-dashboards",
"version": "1.0.0.0",
"version": "1.0.1.0",
"description": "OpenSearch Dashboards Reports Plugin",
"license": "Apache-2.0",
"main": "index.ts",
Expand Down
2 changes: 1 addition & 1 deletion dashboards-reports/server/utils/validationHelper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export const isValidRelativeUrl = (relativeUrl: string) => {
export const regexDuration = /^(-?)P(?=\d|T\d)(?:(\d+)Y)?(?:(\d+)M)?(?:(\d+)([DW]))?(?:T(?:(\d+)H)?(?:(\d+)M)?(?:(\d+(?:\.\d+)?)S)?)?$/;
export const regexEmailAddress = /\S+@\S+\.\S+/;
export const regexReportName = /^[\w\-\s\(\)\[\]\,\_\-+]+$/;
export const regexRelativeUrl = /^\/(_plugin\/kibana\/app|app)\/(dashboards|visualize|discover|notebooks-dashboards\?view=output_only)(\?security_tenant=.+|)#\/(view\/|edit\/)?[^\/]+$/;
export const regexRelativeUrl = /^\/(_dashboards\/app|app)\/(dashboards|visualize|discover|notebooks-dashboards\?view=output_only)([?&]security_tenant=.+|)#\/(view\/|edit\/)?[^\/]+$/;
Copy link
Member

Choose a reason for hiding this comment

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

why is this (_dashboards\/app|app) instead of the (_plugin\/kibana\/|_dashboards\/)?app in main branch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am going off the state of the file from the last PR we pushed to fix the notebooks context menu bug #118. For the PR #132 where the contents were changed to (_plugin\/kibana\/|_dashboards\/)?app, was that to patch for the same bug? If not, I don't think it should go into the patch release unless it's resolving another similar breaking issue?

Copy link
Member

Choose a reason for hiding this comment

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

i see, could you link issue and commit you picked in the description? thanks

Copy link
Member

Choose a reason for hiding this comment

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

it's better we should keep the commit history for patch, right? Can you not co-author on one commit, just keep the original one and add a new commit which only bumps version. Then we do a merge or rebase&merge on github?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently we are co-authored on the first commit not including the version changes or workflow change, I just had to do some manual work there because the cherry-pick failed for some reason even after fetching


export const validateReport = async (
client: ILegacyScopedClusterClient,
Expand Down