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

Update base layout and remove unnecessary code #13

Merged
merged 2 commits into from May 29, 2019

Conversation

Korusuke
Copy link
Member

Fixes #4

@Steap
Copy link
Collaborator

Steap commented May 23, 2019

It is kind of weird to have a commit titled "Merge branch 'master' into base_port". What happened?

You could add "Fixes #4" at the end of the commit log. I think it should close the corresponding issue automagically once we merge the PR.

@reneeotten
Copy link
Contributor

@Korusuke we don't want to have "Merge" commits in the git history.

You probably did git pull; instead you should use git pull --rebase which will add your new commits on top of the master branch. Please see the documentation: https://trac.macports.org/wiki/WorkingWithGit#updating.

Try to recover from this first (a good way of working on your git-vodoo a bit more) and then we can review the actual content. Do make a backup first (git checkout base_port ; git checkout -b backup, go back to your branch git checkout base_port and try to fix it).

@Korusuke
Copy link
Member Author

A merge conflict was showing up due to the newly merged #12 so I just tried solving it using the git GUI which now I guess I shouldn't have done, will fix it in few minutes.

@reneeotten
Copy link
Contributor

A merge conflict was showing up due to the newly merged #12 so I just tried solving it using the git GUI which now I guess I shouldn't have done, will fix it in few minutes.

something is not right yet as you have duplicate fields in the template now....

@Korusuke Korusuke force-pushed the base_port branch 2 times, most recently from f86a41a to a678e31 Compare May 24, 2019 09:01
Copy link
Contributor

@reneeotten reneeotten left a comment

Choose a reason for hiding this comment

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

a suggestions to follow-up on what @Steap already mentioned, besides that it looks good to go!

upt_macports/templates/base.Portfile Outdated Show resolved Hide resolved
upt_macports/templates/base.Portfile Outdated Show resolved Hide resolved
platforms darwin
# uncomment and edit the following line depending on whether the port installs architecture-dependent files
# uncomment the following line if no architecture-dependent files are installed, otherwise remove
Copy link
Contributor

Choose a reason for hiding this comment

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

add "TODO", like: # TODO: uncomment the .....

upt_macports/templates/base.Portfile Outdated Show resolved Hide resolved
@Korusuke
Copy link
Member Author

I have done the necessary changes and will update the Portfile over at macports/ports once this PR is merged.

@reneeotten
Copy link
Contributor

this looks good to me and it can always be fine-tuned based on input from the MacPorts community later. @Steap, since it's really Portfile/MacPorts related and your suggestions incorporated I'm going to assume that your fine with me merging it ;)

@reneeotten reneeotten merged commit e3905d6 into macports:master May 29, 2019
@@ -4,16 +4,22 @@ PortSystem 1.0
{% block portgroup %}
{% endblock %}

# Portfile generated/maintained using the Universal Packaging Tool (upt)
Copy link
Member

Choose a reason for hiding this comment

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

I would probably use maintained only after we get automatic updates working :)

Copy link
Contributor

Choose a reason for hiding this comment

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

we are optimistic about it ;)

# supported_archs noarch
license {{ pkg.licenses }}
# maintainers {example.org:julesverne @GitHub-handle} openmaintainer # TODO
Copy link
Member

Choose a reason for hiding this comment

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

I would also keep a line for nomaintainer and let the user decide between the two.
It doesn't serve anyone to force users to assume maintainership when they are not comfortable to do so and not willing to take care of the port later.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, that's a valid point - how about something like this?

# TODO: either add your e-mail address and GitHub handle or set to nomaintainer
# maintainers         {example.org:julesverne @GitHub-handle} openmaintainer
# maintainers         nomaintainer

Copy link
Member

Choose a reason for hiding this comment

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

One further idea. At the end the port would ideally not contain any comments. So maybe the TODOs are a bit superfluous. Maybe something along those lines?

## If you would like to keep maintaining this port in the future as described in
##     https://guide.macports.org/chunked/project.contributing.html
## please add your contacts (e-mail, github handle), else keep `nomaintainer`.
# maintainers         {example.org:julesverne @github-handle} openmaintainer
# maintainers         nomaintainer

And then also ask users in a comment somewhere on top to go through the commented-out sections and check them out (and ideally remove most comments).

@Korusuke Korusuke deleted the base_port branch May 29, 2019 08:38
@Korusuke Korusuke restored the base_port branch May 30, 2019 10:03
@Korusuke Korusuke deleted the base_port branch May 31, 2019 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

layout base.Portfile
4 participants