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

Master Update Submodules #516

Merged
merged 5 commits into from
Aug 29, 2023

Conversation

Michael7371
Copy link
Contributor

PR Details

Description

Updating submodules using this command: git submodule update --remote --merge

  • asn1_codec: Updating the commit reference from b73989 to commit 792abc0.
  • jpo-cvdp: Updating the commit reference from fa9c0ab to the commit 4d1ba2e.
  • jpo-s3-deposit: Updating the commit reference from 2880f82 to commit 4566ed1.
  • jpo-sdw-depositor: Updating the commit reference from be99dcc to commit 00c070f.
  • jpo-security-svcs: Updating the commit reference from 2c30cd2 to commit 7e6a733.

Related Issue

N/A

Motivation and Context

Includes many changes, one of which is that this commit includes the J2735 ASN file. Also noticed that many other modules were out of date.

How Has This Been Tested?

The submodules updated have already been thoroughly tested, this just updates the reference to those modules.

Types of changes

  • Defect fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that cause existing functionality to change)
  • Update submodule references

Checklist:

  • I have added any new packages to the sonar-scanner.properties file
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
    ODE Contributing Guide
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@paulbourelly999 paulbourelly999 requested review from paulbourelly999 and removed request for dan-du-car August 24, 2023 15:18
@paulbourelly999
Copy link
Contributor

paulbourelly999 commented Aug 24, 2023

@TonyEnglish @Michael7371 Discovered issue with this fork. Was able to build docker images successfully by using git clone --recurse-submodules but ran into issue running kafka container from docker compose. Issue is related to https://github.com/Michael7371/jpo-ode/blob/47d21847a2f087545d7e9ff367e70b07478c881a/docker-compose.yml#L37
https://github.com/Michael7371/jpo-ode/blob/47d21847a2f087545d7e9ff367e70b07478c881a/sample.env#L33.

Kafka container fails to start with the following docker compose error :

Error response from daemon: invalid volume specification: 'C:/var/run/docker.sock:/var/run/docker.sock:rw'

After removing ${DOCKER_SHARED_VOLUME_WINDOWS} from volume declaration in docker compose the issue goes away. There is no README instruction to edit this environment variable at all in installation process so maybe the README of the sample.env needs to be updated. What do you guys think? If this is not concerning I can approve the PR.
NOTE: Only tested docker build and docker compose up. Please let me know if you want me to test specific functionality

Copy link
Contributor

@paulbourelly999 paulbourelly999 left a comment

Choose a reason for hiding this comment

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

See comment about docker-compose.yml and .env file

Copy link
Contributor

@paulbourelly999 paulbourelly999 left a comment

Choose a reason for hiding this comment

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

Looks good.

@paulbourelly999
Copy link
Contributor

@SaikrishnaBairamoni looks like there is a missing SONAR TOKEN. Are you working on this?

@TonyEnglish
Copy link
Contributor

TonyEnglish commented Aug 28, 2023 via email

@SaikrishnaBairamoni
Copy link
Contributor

@SaikrishnaBairamoni looks like there is a missing SONAR TOKEN. Are you working on this?

I've updated the SONAR_TOKEN with new one but seems like its failing due to the sonar cloud organization name in pom.xml
its pointing to usdot-jpo-ode instead of usdot-jpo-ode-1

@Michael7371
Copy link
Contributor Author

@SaikrishnaBairamoni I think I have that updated now, I also pushed that update to the dev pr.

@SaikrishnaBairamoni
Copy link
Contributor

@SaikrishnaBairamoni I think I have that updated now, I also pushed that update to the dev pr.

@Michael7371 Sorry for the inconvenience we have duplicate sonar cloud organization which is usdot-jpo-ode-1 and there is already old account usdot-jpo-ode. we decided to go with old organization and remove the duplicate.

we need to point back Pom.xml file changes from usdot-jpo-ode-1 to usdot-jpo-ode and in ci.yml as below

mvn -e -X clean org.jacoco:jacoco-maven-plugin:prepare-agent package sonar:sonar -Dsonar.projectKey=usdot.jpo.ode:jpo-ode -Dsonar.projectName=jpo-ode -Dsonar.organization=usdot-jpo-ode -Dsonar.host.url=https://sonarcloud.io -Dsonar.branch.name=$GITHUB_REF_NAME

@Michael7371
Copy link
Contributor Author

@SaikrishnaBairamoni I think I have that updated now, I also pushed that update to the dev pr.

@Michael7371 Sorry for the inconvenience we have duplicate sonar cloud organization which is usdot-jpo-ode-1 and there is already old account usdot-jpo-ode. we decided to go with old organization and remove the duplicate.

we need to point back Pom.xml file changes from usdot-jpo-ode-1 to usdot-jpo-ode and in ci.yml as below

mvn -e -X clean org.jacoco:jacoco-maven-plugin:prepare-agent package sonar:sonar -Dsonar.projectKey=usdot.jpo.ode:jpo-ode -Dsonar.projectName=jpo-ode -Dsonar.organization=usdot-jpo-ode -Dsonar.host.url=https://sonarcloud.io -Dsonar.branch.name=$GITHUB_REF_NAME

@SaikrishnaBairamoni Okay that should be updated now.

@paulbourelly999
Copy link
Contributor

@SaikrishnaBairamoni I am not able to merge this PR. Could you give me privileges

@SaikrishnaBairamoni SaikrishnaBairamoni merged commit 4b91f4a into usdot-jpo-ode:master Aug 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants