[NO MERGE] [For Review] prepare for moving into central repro

Azure / azure-event-hubs-node

Node client library for Azure Event Hubs https://azure.microsoft.com/services/event-hubs

MIT License

50 stars 46 forks source link

[NO MERGE] [For Review] prepare for moving into central repro #193

Closed jeremymeng closed 5 years ago

jeremymeng commented 5 years ago

/cc @ramya-rao-a

amarzavery commented 5 years ago

OPS Build status updates of commit b702433:

:clock10: Preparing: average preparing time is 57 sec(s)

amarzavery commented 5 years ago

OPS Build status updates of commit b702433:

:x: Validation status: errors

Please follow instructions here which may help to resolve issue.

File	Status	Preview URL	Details
[]()	:x:Error		Details

[]()

[Error] Publish service's config file: .openpublishing.publish.config.json is not found. Exception:

For more details, please refer to the build report.

Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

ramya-rao-a commented 5 years ago

Thanks @jeremymeng! cc @bterlson

ramya-rao-a commented 5 years ago

This is slightly tough to review as well because of https://github.com/Azure/azure-event-hubs-node/pull/193/commits/e754570c5106ce8b83ae78155dc624a6c90d0db4

@bterlson I guess we will have review each commit separately before and after the above commit

jeremymeng commented 5 years ago

This is slightly tough to review as well because of e754570

Sorry for that. I tried to rebase/shuffle/squash changes, but the renaming made them hard to do without re-doing the changes.

ramya-rao-a commented 5 years ago

Hmmm... we can take this branch, undo the commits till https://github.com/Azure/azure-event-hubs-node/commit/e754570c5106ce8b83ae78155dc624a6c90d0db4, push to new branch, create a new PR.

Then another for just the https://github.com/Azure/azure-event-hubs-node/commit/e754570c5106ce8b83ae78155dc624a6c90d0db4.

Then another for the remaining commits.

Thoughts?

AlexGhiondea commented 5 years ago

@jeremymeng Let's try and have multiple smaller PRs in this repo to make it easier for the reviewers to look at the code. We don't have a lot of experience in this repo and making smaller incremental changes are better.

Once we merge all of those PRs (that move this repo's build system to the one we use in the central repo) we can do a one time move of the repo over -- that will be easier to review (and is what we did for Service Bus and what we want to use as a pattern for how to move repos going forward).

jeremymeng commented 5 years ago

@AlexGhiondea sounds good. Did you create a a new target branch in the old repo for the service bus PR? or is it okay to merge the changes into master? If we want the former, does anyone has permission to create a branch in this repo?

jeremymeng commented 5 years ago

Abandoning in favor of smaller PRs

ramya-rao-a commented 5 years ago

Did you create a a new target branch in the old repo for the service bus PR or is it okay to merge the changes into master?

No, we merged to master.

If we want the former, does anyone has permission to create a branch in this repo?

Can you elaborate on what you mean by the "former"?

jeremymeng commented 5 years ago

Can you elaborate on what you mean by the "former"?

Thanks! I saw the changes in the master. I was under an incorrect impression that we don't want to change master thus suggesting PR against a different branch.