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

[SCB-935] Saga transaction management console UI initial commit #317

Merged
merged 11 commits into from Dec 14, 2018

Conversation

anvithks
Copy link
Contributor

@anvithks anvithks commented Sep 28, 2018

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Make sure there is a JIRA issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a JIRA issue. Your pull request should address just this issue, without pulling in other changes.
  • Each commit in the pull request should have a meaningful subject line and body.
  • Format the pull request title like [SCB-XXX] Fixes bug in ApproximateQuantiles, where you replace SCB-XXX with the appropriate JIRA issue.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Run mvn clean install to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

//TODO

  • Apache license compatibility check
  • Saga Web API server
  • List transaction tracing

@coveralls
Copy link

coveralls commented Sep 28, 2018

Coverage Status

Coverage increased (+0.3%) to 92.097% when pulling 4b2b65f on anvithks:frontend-dev into bb34ce7 on apache:master.

@WillemJiang
Copy link
Member

@anvithks After went through the PR, I found you may just copy https://github.com/akveo/ngx-admin/tree/starter-kit code here. I'm not sure if it's best way to do it, but need to make sure it works with Apache License first.

@WillemJiang
Copy link
Member

BTW, we can need to create a module of UI first, then put the compiled js into the static resource directory of the saga-web.

@anvithks
Copy link
Contributor Author

@anvithks After went through the PR, I found you may just copy https://github.com/akveo/ngx-admin/tree/starter-kit code here. I'm not sure if it's best way to do it, but need to make sure it works with Apache License first.

@WillemJiang sorry for the delayed response. I was on planned vacation and could not check on this.
Yes, you are right we can use the starter kit but it does not have many of the UI components that we have used in the UI. I will check if I can start with the starter-kit and add our modules. Let me get back to you on that.

@anvithks
Copy link
Contributor Author

BTW, we can need to create a module of UI first, then put the compiled js into the static resource directory of the saga-web.

I will write an install script for the UI. This script will compile the UI from the source and copy the contents of the dist/ folder in to the static/ folder.

@asifdxtreme
Copy link
Member

// TODO (Mandatory before the PR merge)

For New dependencies following things should be done

  • Add the license copies in this directory
  • Add the Usage information over here
  • Add the Usage information with the version number over here

@asifdxtreme
Copy link
Member

@anvithks please replace favicon.* with ServiceComb icons, you can get it in this project

@anvithks
Copy link
Contributor Author

@anvithks After went through the PR, I found you may just copy https://github.com/akveo/ngx-admin/tree/starter-kit code here. I'm not sure if it's best way to do it, but need to make sure it works with Apache License first.

@WillemJiang sorry for the delayed response. I was on planned vacation and could not check on this.
Yes, you are right we can use the starter kit but it does not have many of the UI components that we have used in the UI. I will check if I can start with the starter-kit and add our modules. Let me get back to you on that.

@WillemJiang after checking the code base of the starter kit and the one in the commit I find that I am using even lesser modules and components than the starter-kit. I have further reduced the LOC by removing some more unused files.

@WillemJiang
Copy link
Member

@anvithks After went through the PR, I found you may just copy https://github.com/akveo/ngx-admin/tree/starter-kit code here. I'm not sure if it's best way to do it, but need to make sure it works with Apache License first.

@WillemJiang sorry for the delayed response. I was on planned vacation and could not check on this.
Yes, you are right we can use the starter kit but it does not have many of the UI components that we have used in the UI. I will check if I can start with the starter-kit and add our modules. Let me get back to you on that.

@WillemJiang after checking the code base of the starter kit and the one in the commit I find that I am using even lesser modules and components than the starter-kit. I have further reduced the LOC by removing some more unused files.

@anvithks Could you remove the unused file first?

@anvithks
Copy link
Contributor Author

@WillemJiang It's done.

@@ -0,0 +1,75 @@
import { ModuleWithProviders, NgModule, Optional, SkipSelf } from '@angular/core';
Copy link
Member

Choose a reason for hiding this comment

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

I need to know if this file is generated or write by hand.
If it is write by hand we need to apply the apache License header here.
If not, we should find a right way to specify the license.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the files with appropriate license information.

@asifdxtreme
Copy link
Member

@anvithks Please add ASF headers to all the files written by you, for format you can refer to https://github.com/apache/servicecomb-service-center/blob/master/frontend/app/index.html#L1

@WillemJiang
Copy link
Member

@anvithks @asifdxtreme We are plan to do the release of ServiceComb at the end of this month, Please update the License header ASAP.

@WillemJiang
Copy link
Member

@anvithks As there are bunch of code in this PR, do you mind fill an iCLA first. I'd happy merge it into the repository once receiving the iCLA notification.

@asifdxtreme
Copy link
Member

asifdxtreme commented Dec 11, 2018

@anvithks As there are bunch of code in this PR, do you mind fill an iCLA first. I'd happy merge it into the repository once receiving the iCLA notification.

I remember I have already sent ICLA of anvith sometime back

@WillemJiang
Copy link
Member

@anvithks As there are bunch of code in this PR, do you mind fill an iCLA first. I'd happy merge it into the repository once receiving the iCLA notification.

I remember I have already sent ICLA of anvith sometime back

I found the iCLA acknowledge mail. The PR is good to go.

@anvithks
Copy link
Contributor Author

// TODO (Mandatory before the PR merge)

For New dependencies following things should be done

  • [*] Add the license copies in this directory
  • [*] Add the Usage information over here
  • [*] Add the Usage information with the version number over here

@asifdxtreme these are fixed

@asifdxtreme asifdxtreme changed the title [SCB-935] [WIP] Saga transaction management console UI initial commit [SCB-935] Saga transaction management console UI initial commit Dec 11, 2018
@@ -0,0 +1,21 @@
The MIT License (MIT)
Copy link
Member

Choose a reason for hiding this comment

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

@anvithks We can remove this license now, this will now be added as a part of release/license

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@asifdxtreme This is fixed

@asifdxtreme
Copy link
Member

@WillemJiang After running RAT tool on this code below are the list of files which shows abnormal license eventhough licenses has been added in *.scss files

saga-web/src/main/resources/saga-frontend/README.md
saga-web/src/main/resources/saga-frontend/angular.json
saga-web/src/main/resources/saga-frontend/package.json
saga-web/src/main/resources/saga-frontend/tsconfig.json
saga-web/src/main/resources/saga-frontend/src/index.html
saga-web/src/main/resources/saga-frontend/src/tsconfig.app.json
saga-web/src/main/resources/saga-frontend/src/app/@theme/components/footer/footer.component.scss
saga-web/src/main/resources/saga-frontend/src/app/@theme/components/header/header.component.scss
saga-web/src/main/resources/saga-frontend/src/app/@theme/components/switcher/switcher.component.scss
saga-web/src/main/resources/saga-frontend/src/app/@theme/components/theme-settings/theme-settings.component.scss
saga-web/src/main/resources/saga-frontend/src/app/@theme/components/theme-switcher/theme-switcher.component.scss
saga-web/src/main/resources/saga-frontend/src/app/@theme/components/theme-switcher/themes-switcher-list/theme-switcher-list.component.scss
saga-web/src/main/resources/saga-frontend/src/app/@theme/layouts/one-column/one-column.layout.scss
saga-web/src/main/resources/saga-frontend/src/app/@theme/styles/bootstrap-rtl.scss
saga-web/src/main/resources/saga-frontend/src/app/@theme/styles/font-size.scss
saga-web/src/main/resources/saga-frontend/src/app/@theme/styles/pace.theme.scss
saga-web/src/main/resources/saga-frontend/src/app/@theme/styles/styles.scss
saga-web/src/main/resources/saga-frontend/src/app/@theme/styles/themes.scss
saga-web/src/main/resources/saga-frontend/src/app/pages/miscellaneous/not-found/not-found.component.scss
saga-web/src/main/resources/saga-frontend/src/assets/images/square_pattern.svg
saga-web/src/main/resources/saga-frontend/src/assets/images/square_pattern_cosmic.svg

Please confirm if we can go ahead with it?

@WillemJiang
Copy link
Member

I think we can leave the json and svg files there, but I'm not sure about the scss files.
@asifdxtreme Do you have any idea about that?

@asifdxtreme
Copy link
Member

I think we can leave the json and svg files there, but I'm not sure about the scss files.
@asifdxtreme Do you have any idea about that?

@WillemJiang these scss files have the License but RAT tool is not able to detect it

@asifdxtreme
Copy link
Member

asifdxtreme commented Dec 12, 2018

@WillemJiang The Rat report after running it on latest changes is here

/saga-web/src/main/resources/saga-frontend/README.md
/saga-web/src/main/resources/saga-frontend/angular.json
/saga-web/src/main/resources/saga-frontend/package.json
/saga-web/src/main/resources/saga-frontend/tsconfig.json
/saga-web/src/main/resources/saga-frontend/src/tsconfig.app.json
/saga-web/src/main/resources/saga-frontend/src/assets/images/square_pattern.svg
/saga-web/src/main/resources/saga-frontend/src/assets/images/square_pattern_cosmic.svg

@@ -1,7 +1,7 @@
/**
* @license
* Copyright Akveo. All Rights Reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
* Licensed under the MIT License. See licenses/LICENSE-ngxadmin for license information.
Copy link
Member

Choose a reason for hiding this comment

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

I don't think the reference of LICENSE is clean enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@WillemJiang lets discuss how best we can do this. I will make the changes as needed in the next PR.

@WillemJiang
Copy link
Member

WillemJiang commented Dec 14, 2018

As we need to do some clean up of the saga-pack repo. I merged the PR first, we can polish the License header in another PR.

@anvithks Please put the JIRA ID in the comments next for tracking the committer in a better way.
Now I had to squash your commit to update the commit message.

@WillemJiang WillemJiang merged commit 96b1514 into apache:master Dec 14, 2018
@anvithks
Copy link
Contributor Author

As we need to do some clean up of the saga-pack repo. I merged the PR first, we can polish the License header in another PR.

@anvithks Please put the JIRA ID in the comments next for tracking the committer in a better way.
Now I had to squash your commit to update the commit message.

@WillemJiang Did you mean adding JIRA ID to commit messages or any further comments?
I will do the needful.

@WillemJiang
Copy link
Member

@anvithks I mean adding the JIRA ID to the first line of commit log, in this way we could grep the committees which is related to the JIRA ID. It's quite useful when we need to cherry pick different patches between different branches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants