-
Notifications
You must be signed in to change notification settings - Fork 7
drop concurrency api check for performance #58
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
Conversation
- added spinner - more consistent logging & colors - highlight colors - central log.js
it will later handle the concurrency error anyway
|
Needs a rebase. |
|
@rabbah Thanks for the reviews! Regarding the conflict: that would be entirely avoidable if (true) merging would be allowed on PRs: this PR was simply on top of #56 and if that one would have been merged instead of rebased, git would know and this here would not show as a conflict. However, the repo settings here prevent one from choosing merge as an option. I assume that's an Openwhisk project policy? Could it be allowed? I agree with preferring clean histories without a lot of "merge" commits on master. However, in some cases like this it's not avoidable and one is loosing a major git feature if merging is entirely prevented. In our in-house development we address this by allowing all options on PRs, but ask people to avoid merging unless necessary. This way you still avoid the majority of merge commits on master. I guess I should bring this to the dev list? /cc @dgrove-oss |
|
The project uses linear github histories and we avoid merges vs rebase/squash and merge. We can do rebase and merge as oppose to squash and merge if you indicate that in the PR. Otherwise the default is squash and merge. |
|
Sure, but that policy leads to more work in case one has concurrent branches like here :-) Neither rebase nor squash would have helped here, only a proper merge. It seems not an efficient use of git to generally prevent that. |
Follow up from #45 and #32. We can save some time (~800ms in my tests) by not making a request to
/api/v1/api-docsto detect whether the openwhisk system supports concurrency or not. This check doesn't always work and we handle it later gracefully since #32 anyway.Note: this is on top of PR #56, see here for the actual changeset: console-spinner...optimistic-concurrency