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

Sync script for updating frontend #6

Merged
merged 17 commits into from
Dec 17, 2020
Merged

Sync script for updating frontend #6

merged 17 commits into from
Dec 17, 2020

Conversation

alexanderzobnin
Copy link
Contributor

This PR adds some helpful scripts for syncing changes with upstream changes in open source Grafana. It's a bit tricky since we have slightly different code (imports, some dependencies), but looks promising. Script gets a latest changes in the elasticsearch data source folder and then formats it as a git patch. Changes only fetched from the last sync (last sync commit hash is stored in the .upstream.lock file). In most of cases git patch fails, but I use 3 way merge, so changes can be reviewed and applied manually. Example of usage:

alex@alex-ws:es-open-distro-datasource$ ./sync-changes.sh 
Switched to and reset branch 'sync-upstream'
Creating new branch
Switched to a new branch 'sync-upstream-f1b3c8ba4b9f2a3f3f35d8dd092de541784eac67'
Syncing changes with base commit
Getting list of files to sync (ref f1b3c8ba4b9f2a3f3f35d8dd092de541784eac67)
Downloading files
[sync-upstream-f1b3c8ba4b9f2a3f3f35d8dd092de541784eac67 0592813] Sync changes with commit f1b3c8ba4b9f2a3f3f35d8dd092de541784eac67
 25 files changed, 186 insertions(+), 119 deletions(-)
 create mode 100644 src/README.md
 create mode 100644 src/img/elasticsearch.svg
 rewrite src/plugin.json (65%)
Syncing changes with master
Master branch is on f4fcd875c16c67eff36b0535d556417c54e53399
Getting list of files to sync (ref master)
Downloading files
Writing master commit into lockfile
Creating a patch
Cleaning up and applying patch
Switched to branch 'sync-upstream'
error: patch failed: .upstream.lock:1
Falling back to three-way merge...
Applied patch to '.upstream.lock' cleanly.
error: patch failed: src/datasource.ts:9
Falling back to three-way merge...
Applied patch to 'src/datasource.ts' cleanly.
Deleted branch sync-upstream-f1b3c8ba4b9f2a3f3f35d8dd092de541784eac67 (was 0592813).
Patch applied. Now you can review changes and apply it manually if patch failed.

Copy link
Member

@Elfo404 Elfo404 left a comment

Choose a reason for hiding this comment

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

Took this for a spin and seems to work like a charm.

I think is an awesome starting point, left a couple of comments just to think about a couple of points, but IMO it's good to go.

package.json Outdated
@@ -48,5 +48,8 @@
},
"resolutions": {
"rxjs": "6.6.3"
},
"dependencies": {
"request-promise-native": "^1.0.9"
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this be a dev dependency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure. This required for running script. I'll check it out.

@@ -0,0 +1 @@
f1b3c8ba4b9f2a3f3f35d8dd092de541784eac67
Copy link
Member

Choose a reason for hiding this comment

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

maybe we can start with bb45f5fedc (which is the react migration commit) as we did a few changes both here and in upstream. What do you think?


const GH_OWNER = 'grafana';
const GH_REPO = 'grafana';
const ES_DIR_PATH = 'public/app/plugins/datasource/elasticsearch';
Copy link
Member

Choose a reason for hiding this comment

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

I was thinking that instead of this we might want a config file with local/path <-> upstram/path for each file/dir we need to sync, so that this would also take care of dependencies for example. wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, sounds good. Maybe we can reorganize files a bit to make it easier to sync.

@alexanderzobnin alexanderzobnin merged commit c19f66b into master Dec 17, 2020
@Elfo404 Elfo404 deleted the sync-script branch April 14, 2021 08:57
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.

None yet

2 participants