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

change column name type to record_type #10

Merged
merged 1 commit into from
Oct 4, 2019

Conversation

iDevoid
Copy link
Contributor

@iDevoid iDevoid commented Oct 4, 2019

Hi @kcajmagic
I made the changes that you requested on issue #8
What I did:

  • I search type on your repo one by one, what i found that query with column name type is in 2 packages, cassandra and postgresql. I also searched in executor.go files, which have query inside. cassandra executor has no type column, it's already record_type. and in postgresql executor has no column names type or record_type.
  • I changed the json, bson and gorm name which was type at the first to recordtype, since, i don't know if this is needed or not, if it's not needed, i'll change it back to type. because this one is the top json structure, even the name of struct is Record. that doesn't specify the json name. it would look like this
{
    "type": 1,
    "deviceid": 1
}

to

{
    "recordtype": 1,
    "deviceid": 1
}

@CLAassistant
Copy link

CLAassistant commented Oct 4, 2019

CLA assistant check
All committers have signed the CLA.

@iDevoid iDevoid mentioned this pull request Oct 4, 2019
@codecov-io
Copy link

Codecov Report

Merging #10 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #10   +/-   ##
=======================================
  Coverage   57.97%   57.97%           
=======================================
  Files          15       15           
  Lines        1035     1035           
=======================================
  Hits          600      600           
  Misses        433      433           
  Partials        2        2
Impacted Files Coverage Δ
postgresql/db.go 39.47% <100%> (ø) ⬆️
cassandra/db.go 50.56% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dbee34f...2ba4237. Read the comment docs.

@kristinapathak kristinapathak added the hacktoberfest good issue for hacktoberfest label Oct 4, 2019
Copy link
Member

@kcajmagic kcajmagic left a comment

Choose a reason for hiding this comment

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

Awesome, looks good.

@kcajmagic
Copy link
Member

@iDevoid can you please sign the cla so I can merge this PR.

@iDevoid
Copy link
Contributor Author

iDevoid commented Oct 4, 2019

sorry for taking a lot of time to sign it @kcajmagic
It was so hard to access the cla-assistant.io. I got HTTP error 524, I think my internet is bad to access that site.
Thank you for let me working on your repo, I hope I can work on this repo again. 😄 🙏

@kcajmagic kcajmagic merged commit f12f3cf into xmidt-org:master Oct 4, 2019
@kcajmagic
Copy link
Member

@iDevoid, we have been having problems with the assistant the past couple of days too. We love contributors if you would like to help more take a look at some hacktoberfest issues. This one could be fun for you.

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

Successfully merging this pull request may close these issues.

5 participants