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

Test and workflows to check readme file changes on master #210

Closed
wants to merge 4 commits into from

Conversation

sayantani11
Copy link
Contributor

Fixes #177

@sayantani11
Copy link
Contributor Author

I am looking into the error but what I feel is since the superior branch is named 'master', the issue arises as it works for a superior branch named 'main'

@miquelduranfrigola
Copy link
Member

Hi @sayantani11 this is great stuff. Do you think we should rename master to main? (master is old nomenclature in github as you may know). I am afraid of doing so at this stage because many people are currently contributing to ersilia and this may cause problems. What are your thoughts?

@sayantani11
Copy link
Contributor Author

Hi @sayantani11 this is great stuff. Do you think we should rename master to main? (master is old nomenclature in github as you may know). I am afraid of doing so at this stage because many people are currently contributing to ersilia and this may cause problems. What are your thoughts?

Yes, it can create a chaos for the people who are new to git as well. So what I feel is we can hold this for now!! And after 20th we can focus on renaming the branch for this.... As it will be just two more days of the contribution period. What say?

@miquelduranfrigola
Copy link
Member

Completely agree with you. Let's keep this on hold

@sayantani11
Copy link
Contributor Author

@miquelduranfrigola Can you rename the branch?

@GemmaTuron
Copy link
Member

Hi @sayantani11,

We are making some changes to the repository structure, including creating a Docker image for it, so we will rename the branch when we release all these new features. Then we will merge the PR as well.
Sorry this cannot be merged at this moment but many thanks for the work you have done, it will get merged hopefully next week!

@miquelduranfrigola
Copy link
Member

@GemmaTuron this pull request is very interesting. Can we please try to merge it, or at least learn the relevant GitHub Actions contributed?

@miquelduranfrigola
Copy link
Member

@GemmaTuron any progress here?

git fetch origin master:master
git diff --name-only master $HEAD > modified
echo "these files have changed"
cat modified
Copy link

Choose a reason for hiding this comment

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

if this is a huge file , it will basically do a long print with cat, I wouldn't suggest to print it out in a setup as it's not a part of setup.


jobs:
Setup:
runs-on: ubuntu-latest
Copy link

Choose a reason for hiding this comment

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

not sure if this repos maintainers plan to maintain for latest ubuntu or for a specific version. from the docker file it seems to be using ubuntu 20.04.
@miquelduranfrigola : can you confirm against which image the test should be run ?

Copy link
Member

Choose a reason for hiding this comment

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

Hi! ubuntu-latest is good in my opinion!

Copy link
Member

@miquelduranfrigola miquelduranfrigola left a comment

Choose a reason for hiding this comment

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

This looks good! Many thanks!

@miquelduranfrigola
Copy link
Member

@GemmaTuron I am trying to resolve this issue but I am running into conflicts at the moment. Can you please check?

@GemmaTuron
Copy link
Member

Hey, since @sayantani11 is participating in this round, this might be a good moment to close the issue.
@miquelduranfrigola and @sayantani11 I get an error on the Setup of the pull request that says
"Error: The log was not found. It may have been deleted based on retention settings."

@sayantani11
Copy link
Contributor Author

@GemmaTuron @miquelduranfrigola I can actually look into this issue again! And try to figure what's the problem maybe

@miquelduranfrigola
Copy link
Member

Sorry, I overlooked this PR - my apologies. @sayantani11 is doing amazing work on other fronts. So, @sayantani11 - if you have time to work on it, great. If not, I would deprioritise this PR for now.

gitbook-com bot pushed a commit that referenced this pull request Nov 19, 2022
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.

Add workflows and test to check if the writing style guidelines are being followed
4 participants