Skip to content
This repository has been archived by the owner on May 9, 2022. It is now read-only.

Add Cumulocity as Leshan adopter. #86

Merged
merged 1 commit into from
Mar 12, 2021

Conversation

sbernard31
Copy link
Contributor

I would like to add Cumulocity as Leshan adopters. (See eclipse-leshan/leshan#830 (comment))

Cumulocity is done by Ag Software, discussing with us by email they prefer to put forward the "Cumulocity by AG Software" branding, I hope it's OK.

⚠️ I was not able to test this PR locally ⚠️, so please double check it 🙏

The reason : when I changed config\adopters.json file this doesn't changed the UI available at http://localhost:1313/adopters/#iot.leshan when I launch hugo server. I'm not sure If I found something wrong 🐛 or if It's just an issue with npm/node/hugo on my side which I'm not able to explain 🤔 (I tried to clear my browser cache just in case without any effect)

Bonus question, I was understanding that package-lock.json file is intended to be committed and it seems in this project this is not the case, is there any reason ?

@autumnfound
Copy link
Contributor

@sbernard31 You can see the preview here! https://deploy-preview-86--eclipsefdn-adopters.netlify.app/test-new-adopters/#iot.leshan Does this look correct to you? Sometimes we add padding into the logo images to help with ratios

@sbernard31
Copy link
Contributor Author

I think it's OK.

About local testing, is it normal that I didn't succeed to test locally ? or ?

@autumnfound
Copy link
Contributor

I think it's OK.

About local testing, is it normal that I didn't succeed to test locally ? or ?

The issue here seems to be just a miscommunication! The home page will always reflect what is available on production as it uses the main script and hits an API that serves the data to drive this. It is possible to run the entire stack locally, but it is way overkill. We created the page test-new-adopters to enable easier testing, as this reads a local JSON file that has the same data, just without additional context/features (it will always display all as an example). It is built using npm run build iirc, and copies the file into the build directory for you.

@autumnfound
Copy link
Contributor

Also, for the bonus question, it looks like the package-lock.json is committed in the repo. Was it ignored on your side?

@sbernard31
Copy link
Contributor Author

Also, for the bonus question, it looks like the package-lock.json is committed in the repo. Was it ignored on your side?

Uups my mistake it is effectively present. (I guess at some time I see in my git staging files and so I supposed it was not committed, I should have double this check this, sorry about that)

It is possible to run the entire stack locally, but it is way overkill. We created the page test-new-adopters to enable easier testing, as this reads a local JSON file that has the same data, just without additional context/features (it will always display all as an example). It is built using npm run build iirc, and copies the file into the build directory for you.

If I understand, I should run npm run build then go to http://localhost:1313/test-new-adopters ?
If this is correct I guess the README need to be adapted. I just tried it but I have node version issue and no time to dig into this right now.

Naive question, I persist to create PR for my new adopter but maybe this is not really what you expect ? You maybe rather bet that user just open an issue ?

@autumnfound
Copy link
Contributor

If I understand, I should run npm run build then go to http://localhost:1313/test-new-adopters ?
If this is correct I guess the README need to be adapted. I just tried it but I have node version issue and no time to dig into this right now.

Well, npm run build and then hugo server but yes that would be the process! I can look into the README to ensure its up to date. We are looking into the node version stuff as well since newer NPM versions are causing issues. There is a command to downgrade, but we are still looking into it right now.

Naive question, I persist to create PR for my new adopter but maybe this is not really what you expect ? You maybe rather bet that user just open an issue ?

If we need different content, then we would just request an update before we merge. If the content isn't what you expect, then we can assist to try and fudge it to look as the rest do! We have a few tricks that we use to help the consistency of the images. If it's more important that it be up now (maybe because you have something you want to announce and you want something there for it as an added thing), then we can merge and then have an issue to resolve the problem (though it's not ideal).

@sbernard31
Copy link
Contributor Author

We are looking into the node version stuff as well since newer NPM versions are causing issues. There is a command to downgrade, but we are still looking into it right now.

In my case this is a too old version (I'm on debian with the v10.24.0 version packaged in stable repository 😅)

About this PR itself and looking at https://deploy-preview-86--eclipsefdn-adopters.netlify.app/test-new-adopters/#iot.leshan, as I said before I think it OK. So If that sounds good to you too, you can merge it.

In this future instead of trying to build it locally, maybe I should just push the PR check the result looking https://deploy-preview-{$PR_NUMBER}--eclipsefdn-adopters.netlify.app/test-new-adopters/#iot.leshan
And then push force if needed 🤔

@autumnfound
Copy link
Contributor

We are looking into the node version stuff as well since newer NPM versions are causing issues. There is a command to downgrade, but we are still looking into it right now.

In my case this is a too old version (I'm on debian with the v10.24.0 version packaged in stable repository )

About this PR itself and looking at https://deploy-preview-86--eclipsefdn-adopters.netlify.app/test-new-adopters/#iot.leshan, as I said before I think it OK. So If that sounds good to you too, you can merge it.

In this future instead of trying to build it locally, maybe I should just push the PR check the result looking https://deploy-preview-{$PR_NUMBER}--eclipsefdn-adopters.netlify.app/test-new-adopters/#iot.leshan
And then push force if needed

Gotcha! And that is the URL yeah! You can find the URL of the preview site in the checks section under netlify/eclipsefdn-adopters/deploy-preview as well. You can force push or just push a new commit. Both work! We can just squash at the end to make it neat and tidy :)

@autumnfound autumnfound merged commit 97447a4 into EclipseFdn:master Mar 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants