This page is not created by, affiliated with, or supported by Slack Technologies, Inc.
2022-09-23
Channels
- # announcements (8)
- # babashka (12)
- # babashka-sci-dev (6)
- # beginners (62)
- # biff (5)
- # calva (4)
- # cider (2)
- # clj-commons (8)
- # clj-kondo (17)
- # clj-yaml (40)
- # clojars (3)
- # clojure (117)
- # clojure-europe (122)
- # clojure-nl (5)
- # clojure-norway (20)
- # clojurescript (10)
- # consulting (1)
- # datomic (65)
- # events (15)
- # figwheel (1)
- # fulcro (4)
- # lsp (15)
- # mount (15)
- # music (1)
- # off-topic (53)
- # polylith (12)
- # releases (3)
- # shadow-cljs (13)
- # sql (1)
- # test-check (8)
- # xtdb (31)
@grzm Let's continue the discussion here. https://github.com/clj-commons/clj-yaml/pull/38#issuecomment-1256266147 Am I missing something?
Y'all choose. I chose the design I want, understanding the alternatives (even the one you proposed, @borkdude). I've provided my reasoning and believe I've addressed your concerns except for preference. I don't have anything to add. If the consensus of the maintainers is that you want something else, that's the way it should be.
I think I agree 95% with your design, it's just that:
• I think the name could resemble options in similar clojure libraries: unknown-tag-fn
-> default-tag-fn
• Positional arg functions have been known to be cumbersome to work with, if you want to add stuff to them later. So (fn [tag val] ...)
=> (fn [{:keys [tag val]}] ...)
. Unless we're 100% certain that we will never add something to this, a map is the safe, boring choice.
I'm just terrified of adding things that break and using maps has been a good way to prevent that, but maybe I'm exaggerating so I'll be ok with @lee having the final word
Hiya, I’ll catch up in a bit and then share what I think. Hopefully it won’t be a 3rd opinion!
Sorry if any of this has already been sorted…
…agree on no question marks on boolean keyword options… maybe more so for consistency… current clj-yaml keywords booleans do not have question marks… ex. mark
unsafe
…agree that positional args are less extensible/future-proof than a map arg. @grzm are positional args really important to you? Why so strong a position? (pun unintended)
…think that unknown-tag-fn
is probably more self-describing than default-tag-fn
(unless maybe unless you are already familiar with similar clojure libraries?).
@grzm, this has been a bit of a long slog of a PR! I hope you still love us with all of your heart. ❤️
You're misunderstanding. I fear I'm being misunderstood: I don't have a strong position. I view this as a style thing. If my reasons aren't convincing (I believe they're clear), it's a preference thing, and it's up to the maintainers to decide what they want. I don't want to continue discussion that I don't think is worthwhile.
@borkdude are you thinking of cutting a clj-yaml release before snakeyaml 1.33? Or are you going to wait? (I don’t think I can really help Andrey with https://bitbucket.org/snakeyaml/snakeyaml/issues/556/update-yamlloadas-signature-to-support)
I’ll hold off on https://github.com/clj-commons/clj-yaml/issues/47 until I hear back from @slipset. But I’d like to get back to this task, if just to finish it, then all also apply this work to my https://github.com/clj-commons/clj-http-lite/issues/65.
And maybe even also apply it to etaoin and rewrite-clj for clj-commons tag triggered deploy consistency…
So I guess I have some sentimental attachment to Circle. They are/were a Clojure shop, and I’ve been enjoying getting help from and working with @marc-omorain over the years. Where Circle feels like a company, Github and their Actions feels like, eh, Microsoft 🙂
The main reason we’ve been doing stuff with GA at work is exactly to update the github repo, since it feels weird that Circle should go about pushing stuff to github.
So we have amongst other things this:
name: Release
# Controls when the action will run. Workflow runs when manually triggered using the UI
# or API.
on:
workflow_dispatch:
# Inputs the workflow accepts.
inputs:
release_type:
# Friendly description to be shown in the UI instead of 'name'
description: 'Release type, valid values are major, minor, patch'
# Default value if no value is explicitly provided
default: 'patch'
# Input has to be provided for the workflow to run
required: true
skip_tests:
description: 'Should we skip tests on this release?'
default: 'false'
required: false
# A workflow run is made up of one or more jobs that can run sequentially or in parallel
jobs:
# This workflow contains a single job called "greet"
release:
# The type of runner that the job will run on
runs-on: ubuntu-latest
# Steps represent a sequence of tasks that will be executed as part of the job
steps:
- uses: actions/checkout@v2
- name: setup_git
run: |
git config --global user.name '${{ github.actor }}'
git config --global user.email '${{ github.actor }}@users.noreply.github.com'
# Runs a single command using the runners shell
- name: release
run: ./release.sh ${{ github.event.inputs.release_type }}
- name: push
run: git push
at work, which gives us a shiny green button to do deploys from (with some input) from github. Which is nice, since we generally do the squash/merge thing in github.BTW having nvd set up on clj-commons projects would be really nice (and more welcome than switching all stuff to GA 🙂
that would be nice! feel free to @ me for the first few integrations. Please be pedantic (e.g. simply refer to the readme carefully) when setting these up :) often people don't, and end up getting issues (nowadays nvd is a little more proactive against misuse. But still!)
Hmmm.. thinking… So what if clj-commons team documents what it prefers for its projects, and let the primary maintainers choose how to implement those? Maybe things like: • keep a changelog • tag releases • release invocable by maintainer from command line • document release flow and how to release • etc. I could help with this.
@lee @grzm I merged the :unknown-tag-fn
PR by resolving the conflicts locally. I needed to port some markdown to adoc in the process 😅